Re: {painfully BISECTED} Please revert f25c80a4b2: arch/um/drivers:remove duplicate structure field initialization

From: Linus Torvalds
Date: Tue Sep 28 2010 - 16:58:00 EST


On Tue, Sep 28, 2010 at 1:47 PM, Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> And I misread that, assuming that it's just a wrapper around
> eth_mac_addr().  Only it isn't, because it passes eth_mac_addr() the
> MAC address's address directly (with ->sa_data).  But eth_mac_addr()
> expects a `struct sockaddr *'.
>
> And for some wtf reason, eth_mac_addr() passes that `struct
> sockaddr *' in a `void *', thus cunningly hiding the bug.
>
> Yeah, please revert it.

Well, the alternative would be to just fix uml_net_set_mac(). The code
before the patch was clearly crap too, so reverting may make it work,
but gets us back to a stupid situation with two different initializers
for one field.

So perhaps something like the attached patch would make it work, and
have the locking in place that apparently people think it should have?

Boaz?

Linus
arch/um/drivers/net_kern.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/um/drivers/net_kern.c b/arch/um/drivers/net_kern.c
index 2ab233b..e44fd87 100644
--- a/arch/um/drivers/net_kern.c
+++ b/arch/um/drivers/net_kern.c
@@ -257,14 +257,14 @@ static void uml_net_tx_timeout(struct net_device *dev)

static int uml_net_set_mac(struct net_device *dev, void *addr)
{
+ int ret;
struct uml_net_private *lp = netdev_priv(dev);
- struct sockaddr *hwaddr = addr;

spin_lock_irq(&lp->lock);
- eth_mac_addr(dev, hwaddr->sa_data);
+ ret = eth_mac_addr(dev, addr);
spin_unlock_irq(&lp->lock);

- return 0;
+ return ret;
}

static int uml_net_change_mtu(struct net_device *dev, int new_mtu)