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

[Mike Frysinger]

On Thursday 29 January 2009 01:01:41 Ben Warren wrote:
> 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 that's the case, shouldnt we at least mark the README as "these options are 
discouraged / deprecated" ?

i dont have a problem removing the ifndef here as it accomplishes two things:
 - no board_get_enetaddr() reference
 - slightly smaller code

it will break a Blackfin platform or two, but i can fix them up pretty easily.

> >>> +	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.

i have no problem with proactive coding when it makes sense, but i hate some 
of the BSD-ish policies where they try and cram the "n" versions down your 
throat all the time.
-mike