something to warry about...

Johannes Stezenbach js at sig21.net
Tue Jan 18 12:22:12 UTC 2011


On Mon, Jan 17, 2011 at 03:29:20AM +0100, Denys Vlasenko wrote:
> 
> I need a concrete example how to make warning go away in this case:
> 
> static smallint detect_link_priv(void)
> {
>         struct ifreq ifreq;
>         struct mii_ioctl_data *mii = (void *)&ifreq.ifr_data;
> ...
>         mii->reg_num = 1;
> 
> Neither struct ifreq nor struct mii_ioctl_data are defined by busybox -
> they come from kernel.
> The only thing we know about struct ifreq in this code is that
> it contains a char ifr_data[N] vector somewhere inside.

I just looked at /usr/include/linux/if.h and ifr_data is not actually
a char array but a void *.  It doesn't matter wrt the aliasing, but it
might be clearer to use &ifreq.ifr_ifru instead of &ifreq.ifr_data.

The following compiles cleanly (but not tested):

diff --git a/networking/ifplugd.c b/networking/ifplugd.c
index 58f56db..261ee93 100644
--- a/networking/ifplugd.c
+++ b/networking/ifplugd.c
@@ -129,44 +129,49 @@ static int network_ioctl(int request, void* data, const char *errmsg)
 
 /* Link detection routines and table */
 
+union mii_ifreq {
+	struct ifreq ifreq;
+	struct mii_ioctl_data mii;
+};
+
 static smallint detect_link_mii(void)
 {
-	struct ifreq ifreq;
-	struct mii_ioctl_data *mii = (void *)&ifreq.ifr_data;
+	union mii_ifreq mii_ifreq;
+	union mii_ifreq *mii = (void *)&mii_ifreq.ifreq.ifr_data;
 
-	set_ifreq_to_ifname(&ifreq);
+	set_ifreq_to_ifname(&mii_ifreq.ifreq);
 
-	if (network_ioctl(SIOCGMIIPHY, &ifreq, "SIOCGMIIPHY") < 0) {
+	if (network_ioctl(SIOCGMIIPHY, &mii_ifreq, "SIOCGMIIPHY") < 0) {
 		return IFSTATUS_ERR;
 	}
 
-	mii->reg_num = 1;
+	mii->mii.reg_num = 1;
 
-	if (network_ioctl(SIOCGMIIREG, &ifreq, "SIOCGMIIREG") < 0) {
+	if (network_ioctl(SIOCGMIIREG, &mii_ifreq, "SIOCGMIIREG") < 0) {
 		return IFSTATUS_ERR;
 	}
 
-	return (mii->val_out & 0x0004) ? IFSTATUS_UP : IFSTATUS_DOWN;
+	return (mii->mii.val_out & 0x0004) ? IFSTATUS_UP : IFSTATUS_DOWN;
 }
 
 static smallint detect_link_priv(void)
 {
-	struct ifreq ifreq;
-	struct mii_ioctl_data *mii = (void *)&ifreq.ifr_data;
+	union mii_ifreq mii_ifreq;
+	union mii_ifreq *mii = (void *)&mii_ifreq.ifreq.ifr_data;
 
-	set_ifreq_to_ifname(&ifreq);
+	set_ifreq_to_ifname(&mii_ifreq.ifreq);
 
-	if (network_ioctl(SIOCDEVPRIVATE, &ifreq, "SIOCDEVPRIVATE") < 0) {
+	if (network_ioctl(SIOCDEVPRIVATE, &mii_ifreq, "SIOCDEVPRIVATE") < 0) {
 		return IFSTATUS_ERR;
 	}
 
-	mii->reg_num = 1;
+	mii->mii.reg_num = 1;
 
-	if (network_ioctl(SIOCDEVPRIVATE+1, &ifreq, "SIOCDEVPRIVATE+1") < 0) {
+	if (network_ioctl(SIOCDEVPRIVATE+1, &mii_ifreq, "SIOCDEVPRIVATE+1") < 0) {
 		return IFSTATUS_ERR;
 	}
 
-	return (mii->val_out & 0x0004) ? IFSTATUS_UP : IFSTATUS_DOWN;
+	return (mii->mii.val_out & 0x0004) ? IFSTATUS_UP : IFSTATUS_DOWN;
 }
 
 static smallint detect_link_ethtool(void)


This matches my understanding of the C99 standard.  The union type
tells the compiler that struct ifreq and struct mii_ioctl_data can
overlap in the same memory.  And two pointers to the same
type are allowed to alias.


BTW, is there actually any driver which implements link check via
SIOCDEVPRIVATE?  SIOCDEVPRIVATE ioctls have been deprecated for years.


Johannes


More information about the busybox mailing list