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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot