On Wed, Jan 19, 2011 at 1:20 PM, Johannes Stezenbach <[email protected]> wrote: >> > 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; >> >> Here you again assign void** to union mii_ifreq* - >> types still do not match. > > That's why I suggested to replace ifr_data with ifr_ifru, > which is less confusing but the result is the same.
No, it should not be changed. We do not write for compiler. We write for *humans* to read our code. ifr_data isn't called "data" for no reason. It means "a generic data field", and here we use it exactly for that purpose. Whereas ifr_ifru means "ifr union" and is only an implementation detail of the struct ifreq, not meant to be used directly. Had C supported unnamed members like C++ does, struct ifreq would have that member union without name. >> This doesn't emit warnings probably because gcc >> takes a brute-force approach to handling unions >> by marking every pointer derived from any union member >> "potentially aliases anything". Future versions of gcc >> may become "more clever" again, and warnings will return. > > I disagree. C99 aliasing rules say two pointers of the > same type can alias. Consider the follwing: > > void *p = malloc(1000); > union mii_ifreq *m1 = p; > union mii_ifreq *m2 = p + 8; > > m1 and m2 can alias, and this does not change if you > change the line to > > union mii_ifreq *m2 = (void *)&m1->ifreq.ifr_ifru; >> > + union mii_ifreq mii_ifreq; >> > + union mii_ifreq *mii = (void *)&mii_ifreq.ifreq.ifr_data; In your patch, the pointers have an offset relative each other: even if compiler will assume they are aliased, aliasing means mii_ifreq.foo and mii->foo need to be considered as possibly the same. But in your case, mii_ifreq.foo doesn't map to mii->foo, it maps to mii->bar! But I have a deeper problem here. I do not see it as a good decision to butcher a readable piece of code into a obfuscated mess *only* to shut up gcc. gcc has to provide means to do it in some cleaner way. -- vda _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
