Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753306AbbDAMRp (ORCPT ); Wed, 1 Apr 2015 08:17:45 -0400 Received: from arrakis.dune.hu ([78.24.191.176]:52191 "EHLO arrakis.dune.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753029AbbDAMRo (ORCPT ); Wed, 1 Apr 2015 08:17:44 -0400 MIME-Version: 1.0 In-Reply-To: References: <20140624153959.GA19564@google.com> <1403624918.29061.16.camel@joe-AO725> From: Jonas Gorski Date: Wed, 1 Apr 2015 14:17:16 +0200 Message-ID: Subject: Re: [PATCH 1/1] ar7: replace mac address parsing To: Florian Fainelli Cc: Joe Perches , Daniel Walter , Ralf Baechle , Linux-MIPS , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2101 Lines: 56 On Tue, Jun 24, 2014 at 9:26 PM, Florian Fainelli wrote: > 2014-06-24 8:48 GMT-07:00 Joe Perches : >> On Tue, 2014-06-24 at 16:39 +0100, Daniel Walter wrote: >>> Replace sscanf() with mac_pton(). >> [] >>> diff --git a/arch/mips/ar7/platform.c b/arch/mips/ar7/platform.c >> [] >>> @@ -307,10 +307,7 @@ static void __init cpmac_get_mac(int instance, unsigned char *dev_addr) >>> } >>> >>> if (mac) { >>> - if (sscanf(mac, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", >>> - &dev_addr[0], &dev_addr[1], >>> - &dev_addr[2], &dev_addr[3], >>> - &dev_addr[4], &dev_addr[5]) != 6) { >>> + if (!mac_pton(mac, dev_addr)) { >> >> There is a slight functional change with this conversion. >> >> mac_pton is strict about leading 0's and requires a 17 char strlen. > > I do not have my devices handy, but I am fairly positive the use of > sscanf() was exactly for that, we may or may not have leading zeroes. > I am feeling a little uncomfortable with random code changes like that > without being actually able to test on real hardware that has a > variety of bootloaders and environment variables. One of my two devices has a mac address with one of the numbers being < 16, and it uses a fixed length mac: (psbl) printenv ... HWA_0 00:16:B6:2A:A4:3B Also looking at the history[1] of this code, it looks like this was just an optimization of an earlier code which did expect 17 char len: for (i = 0; i < 6; i++) dev_addr[i] = (char2hex(mac[i * 3]) << 4) + char2hex(mac[i * 3 + 1]); So I'm tempted to say it should not cause any issues. But my sample size is rather small. Regards Jonas [1] d16f7093b6eb4f3859856f6ee4ab504cbeeea0b9 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/