[ovs-dev] [PATCH] netdev-linux: Fix strict aliasing warnings.

[Ethan Jackson]

Well it looks good to me.  I'll go ahead and merge it.

Ethan

On Fri, Jan 14, 2011 at 1:04 PM, Ben Pfaff <blp at nicira.com> wrote:
> The normal way to submit that then would be to use --author="Ben Pfaff
> <blp at nicira.com>" to do the commit, instead of adding a tag at the end.
> You can make that change now, by doing "git commit --amend
> --author='...'".
>
> I'm not qualified to review this, since I wrote it.  But you could,
> since you tested it...
>
> On Fri, Jan 14, 2011 at 01:02:28PM -0800, Ethan Jackson wrote:
>> This is exactly the patch Ben submitted in:
>> [ovs-dev] [PATCH 1/2] vswitchd: Add miimon support.
>>
>> I tested it to confirm that it works properly.
>>
>> Ethan
>>
>> On Fri, Jan 14, 2011 at 1:00 PM, Ethan Jackson <ethan at nicira.com> wrote:
>> > This commit fixes warnings caused by the miimon code's breakage of
>> > strict aliasing rules.
>> >
>> > Reported-by: Jesse Gross <jesse at nicira.com>
>> > Implemented-by: Ben Pfaff <blp at nicira.com>
>> > ---
>> > ?lib/netdev-linux.c | ? 45 +++++++++++++++++++++++++++++++--------------
>> > ?1 files changed, 31 insertions(+), 14 deletions(-)
>> >
>> > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
>> > index f953cfc..1ff4d40 100644
>> > --- a/lib/netdev-linux.c
>> > +++ b/lib/netdev-linux.c
>> > @@ -1,5 +1,5 @@
>> > ?/*
>> > - * Copyright (c) 2009, 2010 Nicira Networks.
>> > + * Copyright (c) 2009, 2010, 2011 Nicira Networks.
>> > ?*
>> > ?* Licensed under the Apache License, Version 2.0 (the "License");
>> > ?* you may not use this file except in compliance with the License.
>> > @@ -1009,31 +1009,45 @@ exit:
>> > ?}
>> >
>> > ?static int
>> > -netdev_linux_get_miimon(const struct netdev *netdev_, bool *miimon)
>> > +netdev_linux_do_miimon(const struct netdev *netdev, int cmd,
>> > + ? ? ? ? ? ? ? ? ? ? ? const char *cmd_name, struct mii_ioctl_data *data)
>> > ?{
>> > - ? ?int error;
>> > ? ? struct ifreq ifr;
>> > - ? ?const char *name = netdev_get_name(netdev_);
>> > + ? ?int error;
>> >
>> > - ? ?*miimon = false;
>> > ? ? memset(&ifr, 0, sizeof ifr);
>> > + ? ?memcpy(&ifr.ifr_data, data, sizeof *data);
>> > + ? ?error = netdev_linux_do_ioctl(netdev_get_name(netdev),
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&ifr, cmd, cmd_name);
>> > + ? ?memcpy(data, &ifr.ifr_data, sizeof *data);
>> >
>> > - ? ?error = netdev_linux_do_ioctl(name, &ifr, SIOCGMIIPHY, "SIOCGMIIPHY");
>> > - ? ?if (!error) {
>> > - ? ? ? ?struct mii_ioctl_data *data = (struct mii_ioctl_data *)&ifr.ifr_data;
>> > + ? ?return error;
>> > +}
>> > +
>> > +static int
>> > +netdev_linux_get_miimon(const struct netdev *netdev, bool *miimon)
>> > +{
>> > + ? ?const char *name = netdev_get_name(netdev);
>> > + ? ?struct mii_ioctl_data data;
>> > + ? ?int error;
>> >
>> > - ? ? ? ?/* data->phy_id is filled out by previous SIOCGMIIPHY ioctl call. */
>> > - ? ? ? ?data->reg_num = MII_BMSR;
>> > - ? ? ? ?error = netdev_linux_do_ioctl(name, &ifr, SIOCGMIIREG, "SIOCGMIIREG");
>> > + ? ?*miimon = false;
>> > +
>> > + ? ?memset(&data, 0, sizeof data);
>> > + ? ?error = netdev_linux_do_miimon(netdev, SIOCGMIIPHY, "SIOCGMIIPHY", &data);
>> > + ? ?if (!error) {
>> > + ? ? ? ?/* data.phy_id is filled out by previous SIOCGMIIPHY miimon call. */
>> > + ? ? ? ?data.reg_num = MII_BMSR;
>> > + ? ? ? ?error = netdev_linux_do_miimon(netdev, SIOCGMIIREG, "SIOCGMIIREG",
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &data);
>> >
>> > ? ? ? ? if (!error) {
>> > - ? ? ? ? ? ?*miimon = !!(data->val_out & BMSR_LSTATUS);
>> > + ? ? ? ? ? ?*miimon = !!(data.val_out & BMSR_LSTATUS);
>> > ? ? ? ? } else {
>> > ? ? ? ? ? ? VLOG_WARN_RL(&rl, "%s: failed to query MII", name);
>> > ? ? ? ? }
>> > ? ? } else {
>> > ? ? ? ? struct ethtool_cmd ecmd;
>> > - ? ? ? ?struct ethtool_value *eval = (struct ethtool_value *) &ecmd;
>> >
>> > ? ? ? ? VLOG_DBG_RL(&rl, "%s: failed to query MII, falling back to ethtool",
>> > ? ? ? ? ? ? ? ? ? ? name);
>> > @@ -1042,7 +1056,10 @@ netdev_linux_get_miimon(const struct netdev *netdev_, bool *miimon)
>> > ? ? ? ? error = netdev_linux_do_ethtool(name, &ecmd, ETHTOOL_GLINK,
>> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "ETHTOOL_GLINK");
>> > ? ? ? ? if (!error) {
>> > - ? ? ? ? ? ?*miimon = !!eval->data;
>> > + ? ? ? ? ? ?struct ethtool_value eval;
>> > +
>> > + ? ? ? ? ? ?memcpy(&eval, &ecmd, sizeof eval);
>> > + ? ? ? ? ? ?*miimon = !!eval.data;
>> > ? ? ? ? } else {
>> > ? ? ? ? ? ? VLOG_WARN_RL(&rl, "%s: ethtool link status failed", name);
>> > ? ? ? ? }
>> > --
>> > 1.7.2
>> >
>> >
>> > _______________________________________________
>> > dev mailing list
>> > dev at openvswitch.org
>> > http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
>> >
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
>