[U-Boot] [PATCH 01/27] Blackfin: bfin_mac: force board_get_enetaddr() usage

Ben Warren biggerbadderben at gmail.com
Thu Jan 29 07:01:41 CET 2009


Mike Frysinger wrote:
> On Thursday 29 January 2009 00:43:31 Ben Warren wrote:
>   
>> Mike Frysinger wrote:
>>     
>>> --- a/drivers/net/bfin_mac.c
>>> +++ b/drivers/net/bfin_mac.c
>>>
>>>  	eth_register(dev);
>>>
>>> +	ethaddr = getenv("ethaddr");
>>> +#ifndef CONFIG_ETHADDR
>>>       
>> I know this was there before, but CONFIG_ETHADDR is kinda deprecated.
>> We don't allow it in in-tree config files, so as far as I'm concerned we
>> should pretend it doesn't exist.  Boards should get their MAC address
>> from an EEPROM or from the environment.
>>     
>
> that's news to me.  i see no mention of deprecation in the README file (quite 
> the opposite ... looks fully supported there), and i see a ton of board 
> configs defining it:
> $ grep 'CONFIG_ETH.*ADDR' include/configs/*.h | wc -l
> 257
>
> so what's up ?
>
>   
Hence the 'kinda'.  It's one of those things that doesn't make sense 
(MAC addresses are supposed to be unique, so why would you hard code?) 
and for at least the past year we've been rejecting config files that 
use it.
>>> +	if (ethaddr == NULL) {
>>> +		char nid[20];
>>> +		board_get_enetaddr(bd->bi_enetaddr);
>>> +		sprintf(nid, "%02X:%02X:%02X:%02X:%02X:%02X",
>>>       
>> How about snprintf()
>>     
>
> when would the limit actually be violated ?  bi_enetaddr is unsigned char 
> which means it is impossible for it to be represented as more than two chars.  
> so storage would always be exactly 2 * 6 + 5 + 1 (17 bytes).
>   
You're right, not a strong opinion on my part.  I guess I've been beaten 
to submission by Coverity at work and so always use the 'n' functions.
> -mike
>   



More information about the U-Boot mailing list