Hi Dan,
On Wed, Oct 01, 2014 at 07:54:02PM +0300, Dan Carpenter wrote:
> Hello Antoine Ténart,
>
> The patch 39830689ef0a: "net: pxa168_eth: set the mac address on the
> Ethernet controller" from Sep 30, 2014, leads to the following static
> checker warning:
>
> drivers/net/ethernet/marvell/pxa168_eth.c:640 pxa168_eth_set_mac_address()
> warn: using signed char for bitops
Thanks for reporting this!
>
> drivers/net/ethernet/marvell/pxa168_eth.c
> 625 static int pxa168_eth_set_mac_address(struct net_device *dev, void *addr)
> 626 {
> 627 struct sockaddr *sa = addr;
> 628 struct pxa168_eth_private *pep = netdev_priv(dev);
> 629 unsigned char oldMac[ETH_ALEN];
> 630 u32 mac_h, mac_l;
> 631
> 632 if (!is_valid_ether_addr(sa->sa_data))
> 633 return -EADDRNOTAVAIL;
> 634 memcpy(oldMac, dev->dev_addr, ETH_ALEN);
> 635 memcpy(dev->dev_addr, sa->sa_data, ETH_ALEN);
> 636
> 637 mac_h = sa->sa_data[0] << 24;
> 638 mac_h |= sa->sa_data[1] << 16;
> 639 mac_h |= sa->sa_data[2] << 8;
> 640 mac_h |= sa->sa_data[3];
>
> You may end up with weird signedness bugs doing this (depending of if
> the highest bit is ever used).
Since dev->dev_addr is of type unsigned char *, I could use it instead.
That's what most of the other net drivers do, at least.
While having a look on other net drivers, I spotted
drivers/net/ethernet/toshiba/spider_net.c which is also using signed
char for bitops and may need a fix:
macu = (addr->sa_data[0]<<24) + (addr->sa_data[1]<<16) +
(addr->sa_data[2]<<8) + (addr->sa_data[3]);
macl = (addr->sa_data[4]<<8) + (addr->sa_data[5]);
I'll cook up a fix for both of them if the solution's OK with you.
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
