| Message ID | 1320970267-22297-1-git-send-email-vapier@gentoo.org |
|---|---|
| State | Accepted |
| Delegated to: | Mike Frysinger |
| Headers | show |
On Thu, Nov 10, 2011 at 6:11 PM, Mike Frysinger <vapier@gentoo.org> wrote: > diff --git a/include/net.h b/include/net.h > index ad9afbf..b4acd8f 100644 > --- a/include/net.h > +++ b/include/net.h > @@ -66,8 +66,6 @@ typedef void rxhand_icmp_f(unsigned type, unsigned code, unsigned dport, > */ > typedef void thand_f(void); > > -#define NAMESIZE 16 > - > enum eth_state_t { > ETH_STATE_INIT, > ETH_STATE_PASSIVE, > @@ -75,7 +73,7 @@ enum eth_state_t { > }; > > struct eth_device { > - char name[NAMESIZE]; > + char name[16]; I like all of the earlier NAMESIZE->sizeof changes, but I'm not as comfortable with changing the various name declarations. Seems like we might want named constants for them, just so any dependencies can be clearly-documented. For instance, in Linux the MDIO bus specifiers are designed to be bigger than the MDIO device name, and the size declarations reflect this. Andy
On Friday 11 November 2011 17:07:46 Andy Fleming wrote: > On Thu, Nov 10, 2011 at 6:11 PM, Mike Frysinger <vapier@gentoo.org> wrote: > > diff --git a/include/net.h b/include/net.h > > index ad9afbf..b4acd8f 100644 > > --- a/include/net.h > > +++ b/include/net.h > > @@ -66,8 +66,6 @@ typedef void rxhand_icmp_f(unsigned type, unsigned > > code, unsigned dport, */ > > typedef void thand_f(void); > > > > -#define NAMESIZE 16 > > - > > enum eth_state_t { > > ETH_STATE_INIT, > > ETH_STATE_PASSIVE, > > @@ -75,7 +73,7 @@ enum eth_state_t { > > }; > > > > struct eth_device { > > - char name[NAMESIZE]; > > + char name[16]; > > I like all of the earlier NAMESIZE->sizeof changes, but I'm not as > comfortable with changing the various name declarations. Seems like we > might want named constants for them, just so any dependencies can be > clearly-documented. For instance, in Linux the MDIO bus specifiers are > designed to be bigger than the MDIO device name, and the size > declarations reflect this. sounds like something that can easily be expressed with: BUILD_BUG_ON(sizeof(dev->name) < sizeof(mii->name)); -mike
Dear Mike Frysinger, In message <1320970267-22297-1-git-send-email-vapier@gentoo.org> you wrote: > A few subsystems are using the same define "NAMESIZE". This has been > working so far because they define it to the same number. However, I > want to change the size of eth_device's NAMESIZE, so rather than tweak > the define names, simply drop references to it. Almost no one does, > and the handful that do can easily be changed to a sizeof(). > > Signed-off-by: Mike Frysinger <vapier@gentoo.org> > --- > board/Marvell/db64360/mv_eth.c | 2 +- > board/Marvell/db64460/mv_eth.c | 2 +- > board/esd/cpci750/mv_eth.c | 2 +- > board/evb64260/eth.c | 2 +- > board/prodrive/p3mx/mv_eth.c | 2 +- > drivers/net/armada100_fec.c | 2 +- > drivers/net/mvgbe.c | 2 +- > drivers/qe/uec_phy.c | 2 +- > include/miiphy.h | 2 +- > include/net.h | 4 +--- > include/serial.h | 5 ++--- > net/eth.c | 2 +- > 12 files changed, 13 insertions(+), 16 deletions(-) Applied, thanks. Best regards, Wolfgang Denk
diff --git a/board/Marvell/db64360/mv_eth.c b/board/Marvell/db64360/mv_eth.c index 30304b0..82e08a1 100644 --- a/board/Marvell/db64360/mv_eth.c +++ b/board/Marvell/db64360/mv_eth.c @@ -221,7 +221,7 @@ void mv6436x_eth_initialize (bd_t * bis) return; } - /* must be less than NAMESIZE (16) */ + /* must be less than sizeof(dev->name) */ sprintf (dev->name, "mv_enet%d", devnum); #ifdef DEBUG diff --git a/board/Marvell/db64460/mv_eth.c b/board/Marvell/db64460/mv_eth.c index cd9d5a4..e20ca20 100644 --- a/board/Marvell/db64460/mv_eth.c +++ b/board/Marvell/db64460/mv_eth.c @@ -221,7 +221,7 @@ void mv6446x_eth_initialize (bd_t * bis) return; } - /* must be less than NAMESIZE (16) */ + /* must be less than sizeof(dev->name) */ sprintf (dev->name, "mv_enet%d", devnum); #ifdef DEBUG diff --git a/board/esd/cpci750/mv_eth.c b/board/esd/cpci750/mv_eth.c index 781ad23..06ae6ff 100644 --- a/board/esd/cpci750/mv_eth.c +++ b/board/esd/cpci750/mv_eth.c @@ -221,7 +221,7 @@ void mv6436x_eth_initialize (bd_t * bis) return; } - /* must be less than NAMESIZE (16) */ + /* must be less than sizeof(dev->name) */ sprintf (dev->name, "mv_enet%d", devnum); #ifdef DEBUG diff --git a/board/evb64260/eth.c b/board/evb64260/eth.c index 1492ffc..e96b0f4 100644 --- a/board/evb64260/eth.c +++ b/board/evb64260/eth.c @@ -685,7 +685,7 @@ gt6426x_eth_initialize(bd_t *bis) return; } - /* must be less than NAMESIZE (16) */ + /* must be less than sizeof(dev->name) */ sprintf(dev->name, "gal_enet%d", devnum); #ifdef DEBUG diff --git a/board/prodrive/p3mx/mv_eth.c b/board/prodrive/p3mx/mv_eth.c index 15b3bfc..9e3213b 100644 --- a/board/prodrive/p3mx/mv_eth.c +++ b/board/prodrive/p3mx/mv_eth.c @@ -274,7 +274,7 @@ void mv6446x_eth_initialize (bd_t * bis) return; } - /* must be less than NAMESIZE (16) */ + /* must be less than sizeof(dev->name) */ sprintf (dev->name, "mv_enet%d", devnum); #ifdef DEBUG diff --git a/drivers/net/armada100_fec.c b/drivers/net/armada100_fec.c index fbf9763..0a45f17 100644 --- a/drivers/net/armada100_fec.c +++ b/drivers/net/armada100_fec.c @@ -709,7 +709,7 @@ int armada100_fec_register(unsigned long base_addr) /* Assign ARMADA100 Fast Ethernet Controller Base Address */ darmdfec->regs = (void *)base_addr; - /* must be less than NAMESIZE (16) */ + /* must be less than sizeof(dev->name) */ strcpy(dev->name, "armd-fec0"); dev->init = armdfec_init; diff --git a/drivers/net/mvgbe.c b/drivers/net/mvgbe.c index fd13428..9ce5f6b 100644 --- a/drivers/net/mvgbe.c +++ b/drivers/net/mvgbe.c @@ -700,7 +700,7 @@ error1: dev = &dmvgbe->dev; - /* must be less than NAMESIZE (16) */ + /* must be less than sizeof(dev->name) */ sprintf(dev->name, "egiga%d", devnum); /* Extract the MAC address from the environment */ diff --git a/drivers/qe/uec_phy.c b/drivers/qe/uec_phy.c index e26218b..ac580a0 100644 --- a/drivers/qe/uec_phy.c +++ b/drivers/qe/uec_phy.c @@ -85,7 +85,7 @@ #endif struct fixed_phy_port { - char name[NAMESIZE]; /* ethernet port name */ + char name[16]; /* ethernet port name */ unsigned int speed; /* specified speed 10,100 or 1000 */ unsigned int duplex; /* specified duplex FULL or HALF */ }; diff --git a/include/miiphy.h b/include/miiphy.h index 7e70cf8..ca5040e 100644 --- a/include/miiphy.h +++ b/include/miiphy.h @@ -86,7 +86,7 @@ void mdio_list_devices(void); #define BB_MII_DEVNAME "bb_miiphy" struct bb_miiphy_bus { - char name[NAMESIZE]; + char name[16]; int (*init)(struct bb_miiphy_bus *bus); int (*mdio_active)(struct bb_miiphy_bus *bus); int (*mdio_tristate)(struct bb_miiphy_bus *bus); diff --git a/include/net.h b/include/net.h index ad9afbf..b4acd8f 100644 --- a/include/net.h +++ b/include/net.h @@ -66,8 +66,6 @@ typedef void rxhand_icmp_f(unsigned type, unsigned code, unsigned dport, */ typedef void thand_f(void); -#define NAMESIZE 16 - enum eth_state_t { ETH_STATE_INIT, ETH_STATE_PASSIVE, @@ -75,7 +73,7 @@ enum eth_state_t { }; struct eth_device { - char name[NAMESIZE]; + char name[16]; unsigned char enetaddr[6]; int iobase; int state; diff --git a/include/serial.h b/include/serial.h index 5926244..2b9e647 100644 --- a/include/serial.h +++ b/include/serial.h @@ -3,10 +3,9 @@ #include <post.h> -#define NAMESIZE 16 - struct serial_device { - char name[NAMESIZE]; + /* enough bytes to match alignment of following func pointer */ + char name[16]; int (*init) (void); int (*uninit) (void); diff --git a/net/eth.c b/net/eth.c index 4280d6d..cdc3234 100644 --- a/net/eth.c +++ b/net/eth.c @@ -220,7 +220,7 @@ int eth_register(struct eth_device *dev) { struct eth_device *d; - assert(strlen(dev->name) < NAMESIZE); + assert(strlen(dev->name) < sizeof(dev->name)); if (!eth_devices) { eth_current = eth_devices = dev;
A few subsystems are using the same define "NAMESIZE". This has been working so far because they define it to the same number. However, I want to change the size of eth_device's NAMESIZE, so rather than tweak the define names, simply drop references to it. Almost no one does, and the handful that do can easily be changed to a sizeof(). Signed-off-by: Mike Frysinger <vapier@gentoo.org> --- board/Marvell/db64360/mv_eth.c | 2 +- board/Marvell/db64460/mv_eth.c | 2 +- board/esd/cpci750/mv_eth.c | 2 +- board/evb64260/eth.c | 2 +- board/prodrive/p3mx/mv_eth.c | 2 +- drivers/net/armada100_fec.c | 2 +- drivers/net/mvgbe.c | 2 +- drivers/qe/uec_phy.c | 2 +- include/miiphy.h | 2 +- include/net.h | 4 +--- include/serial.h | 5 ++--- net/eth.c | 2 +- 12 files changed, 13 insertions(+), 16 deletions(-)