[ovs-dev] [PATCH 1/2] vswitchd: Add miimon support.

[Ethan Jackson]

I can only reproduce the warnings with -Wstrict-aliasing=1, the most
aggressive (and prone to false positives) level.  When I run on this
level I find that we break strict aliasing rules in many places
throughout the code base.  Are you configuring with this option?  What
version of gcc are you using?

I think we should either let this go, or change the compiler options
to -Wstrict-aliasing=1 and document in the CodingStyle that we require
strict aliasing to be respected in all cases, fix up the existing
issues, and enforce it as a matter of style.

Ethan

On Wed, Jan 12, 2011 at 5:01 PM, Ethan Jackson <ethan at nicira.com> wrote:
> Oh interesting.  Didn't cause warnings for me.  I'll submit a patch.
>
> Ethan
>
> On Wed, Jan 12, 2011 at 4:55 PM, Jesse Gross <jesse at nicira.com> wrote:
>> On Wed, Jan 12, 2011 at 6:27 PM, Ethan Jackson <ethan at nicira.com> wrote:
>>> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
>>> index 97f9a64..8333bf5 100644
>>> --- a/lib/netdev-linux.c
>>> +++ b/lib/netdev-linux.c
>> [...]
>>> +static int
>>> +netdev_linux_get_miimon(const struct netdev *netdev_, bool *miimon)
>>> +{
>>> +    int error;
>>> +    struct ifreq ifr;
>>> +    const char *name = netdev_get_name(netdev_);
>>> +
>>> +    *miimon = false;
>>> +    memset(&ifr, 0, sizeof ifr);
>>> +
>>> +    error = netdev_linux_do_ioctl(name, &ifr, SIOCGMIIPHY, "SIOCGMIIPHY");
>>> +    if (!error) {
>>> +        struct mii_ioctl_data *data = (struct mii_ioctl_data *)&ifr.ifr_data;
>>> +
>>> +        data->reg_num = MII_BMSR;
>>> +        error = netdev_linux_do_ioctl(name, &ifr, SIOCGMIIREG, "SIOCGMIIREG");
>>> +
>>> +        if (!error) {
>>> +            *miimon = !!(data->val_out & BMSR_LSTATUS);
>>
>> This causes new warnings for me:
>> lib/netdev-linux.c: In function ‘netdev_linux_get_miimon’:
>> lib/netdev-linux.c:1030: warning: dereferencing pointer ‘data’ does
>> break strict-aliasing rules
>> lib/netdev-linux.c:1026: warning: dereferencing pointer ‘data’ does
>> break strict-aliasing rules
>> lib/netdev-linux.c:1023: note: initialized from here
>> lib/netdev-linux.c:1045: warning: dereferencing pointer ‘eval’ does
>> break strict-aliasing rules
>> lib/netdev-linux.c:1036: note: initialized from here
>>
>> You're not allowed to have dereferenced data with two different types.
>>  In this case, the compiler would be within its rights to optimize out
>> the assignment to data->reg_num, since from its perspective nothing
>> ever uses it.
>>
>