2017-04-23 18:02:22

by Karim Eshapa

[permalink] [raw]
Subject: [PATCH 1/1] drivers:net:ethernet:adi:bfin_mac: Use FIELD_SIZEOF defined kernel macro

Use FIELD_SIZEOF defined kernel macro kernel.h

Signed-off-by: Karim Eshapa <[email protected]>
---
drivers/net/ethernet/adi/bfin_mac.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/adi/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
index a9ac58c..60346e0 100644
--- a/drivers/net/ethernet/adi/bfin_mac.c
+++ b/drivers/net/ethernet/adi/bfin_mac.c
@@ -452,10 +452,14 @@ static irqreturn_t bfin_mac_wake_interrupt(int irq, void *dev_id)
static void bfin_mac_ethtool_getdrvinfo(struct net_device *dev,
struct ethtool_drvinfo *info)
{
- strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
- strlcpy(info->version, DRV_VERSION, sizeof(info->version));
- strlcpy(info->fw_version, "N/A", sizeof(info->fw_version));
- strlcpy(info->bus_info, dev_name(&dev->dev), sizeof(info->bus_info));
+ strlcpy(info->driver, KBUILD_MODNAME, FIELD_SIZEOF(
+ struct ethtool_drvinfo, driver));
+ strlcpy(info->version, DRV_VERSION, FIELD_SIZEOF(
+ struct ethtool_drvinfo, version));
+ strlcpy(info->fw_version, "N/A", FIELD_SIZEOF(
+ struct ethtool_drvinfo, fw_version));
+ strlcpy(info->bus_info, dev_name(&dev->dev), FIELD_SIZEOF(
+ struct ethtool_drvinfo, bus_info));
}

static void bfin_mac_ethtool_getwol(struct net_device *dev,
@@ -785,7 +789,7 @@ static int bfin_mac_hwtstamp_get(struct net_device *netdev,
struct bfin_mac_local *lp = netdev_priv(netdev);

return copy_to_user(ifr->ifr_data, &lp->stamp_cfg,
- sizeof(lp->stamp_cfg)) ?
+ FILD_SIZEOF(struct bfin_mac_local, stamp_cfg)) ?
-EFAULT : 0;
}

--
2.7.4


2017-04-23 20:14:22

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/1] drivers:net:ethernet:adi:bfin_mac: Use FIELD_SIZEOF defined kernel macro

Hi Karim,

[auto build test ERROR on net-next/master]
[also build test ERROR on v4.11-rc7 next-20170421]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Karim-Eshapa/drivers-net-ethernet-adi-bfin_mac-Use-FIELD_SIZEOF-defined-kernel-macro/20170424-022248
config: blackfin-BF518F-EZBRD_defconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin

All error/warnings (new ones prefixed by >>):

drivers/net/ethernet/adi/bfin_mac.c: In function 'bfin_mac_hwtstamp_get':
>> drivers/net/ethernet/adi/bfin_mac.c:792:8: error: implicit declaration of function 'FILD_SIZEOF' [-Werror=implicit-function-declaration]
FILD_SIZEOF(struct bfin_mac_local, stamp_cfg)) ?
^~~~~~~~~~~
>> drivers/net/ethernet/adi/bfin_mac.c:792:20: error: expected expression before 'struct'
FILD_SIZEOF(struct bfin_mac_local, stamp_cfg)) ?
^~~~~~
>> drivers/net/ethernet/adi/bfin_mac.c:794:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
cc1: some warnings being treated as errors

vim +/FILD_SIZEOF +792 drivers/net/ethernet/adi/bfin_mac.c

786 static int bfin_mac_hwtstamp_get(struct net_device *netdev,
787 struct ifreq *ifr)
788 {
789 struct bfin_mac_local *lp = netdev_priv(netdev);
790
791 return copy_to_user(ifr->ifr_data, &lp->stamp_cfg,
> 792 FILD_SIZEOF(struct bfin_mac_local, stamp_cfg)) ?
793 -EFAULT : 0;
> 794 }
795
796 static void bfin_tx_hwtstamp(struct net_device *netdev, struct sk_buff *skb)
797 {

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (2.00 kB)
.config.gz (11.87 kB)
Download all attachments

2017-04-23 20:56:50

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 1/1] drivers:net:ethernet:adi:bfin_mac: Use FIELD_SIZEOF defined kernel macro

Hi Karim,

On Sun, Apr 23, 2017 at 8:02 PM, Karim Eshapa <[email protected]> wrote:
> Use FIELD_SIZEOF defined kernel macro kernel.h
>
> Signed-off-by: Karim Eshapa <[email protected]>
> ---
> drivers/net/ethernet/adi/bfin_mac.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/adi/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
> index a9ac58c..60346e0 100644
> --- a/drivers/net/ethernet/adi/bfin_mac.c
> +++ b/drivers/net/ethernet/adi/bfin_mac.c
> @@ -452,10 +452,14 @@ static irqreturn_t bfin_mac_wake_interrupt(int irq, void *dev_id)
> static void bfin_mac_ethtool_getdrvinfo(struct net_device *dev,
> struct ethtool_drvinfo *info)
> {
> - strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
> - strlcpy(info->version, DRV_VERSION, sizeof(info->version));
> - strlcpy(info->fw_version, "N/A", sizeof(info->fw_version));
> - strlcpy(info->bus_info, dev_name(&dev->dev), sizeof(info->bus_info));
> + strlcpy(info->driver, KBUILD_MODNAME, FIELD_SIZEOF(
> + struct ethtool_drvinfo, driver));

IMHO this makes the code less safe and less future-proof.
What if the type of info is ever changed?
There's no safety check to validate that the FIELD_SIZEOF() operates on the
same data as the strlcpy() destination.

> + strlcpy(info->version, DRV_VERSION, FIELD_SIZEOF(
> + struct ethtool_drvinfo, version));
> + strlcpy(info->fw_version, "N/A", FIELD_SIZEOF(
> + struct ethtool_drvinfo, fw_version));
> + strlcpy(info->bus_info, dev_name(&dev->dev), FIELD_SIZEOF(
> + struct ethtool_drvinfo, bus_info));
> }
>
> static void bfin_mac_ethtool_getwol(struct net_device *dev,
> @@ -785,7 +789,7 @@ static int bfin_mac_hwtstamp_get(struct net_device *netdev,
> struct bfin_mac_local *lp = netdev_priv(netdev);
>
> return copy_to_user(ifr->ifr_data, &lp->stamp_cfg,
> - sizeof(lp->stamp_cfg)) ?
> + FILD_SIZEOF(struct bfin_mac_local, stamp_cfg)) ?

As the kbuild test robot already told you, this doesn't compile.
Please try to (at least) compile the code before sending patches.
Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2017-04-24 00:23:29

by Karim Eshapa

[permalink] [raw]
Subject: RE:drivers:net:ethernet:adi:bfin_mac: Use FIELD_SIZEOF defined kernel macro

On Sun, 23 Apr 2017 22:56:38 +0200, Geert Uytterhoeven:
>IMHO this makes the code less safe and less future-proof.
>What if the type of info is ever changed?
>There's no safety check to validate that the FIELD_SIZEOF() operates on the
>same data as the strlcpy() destination.

Really make sense :)

>As the kbuild test robot already told you, this doesn't compile.
>Please try to (at least) compile the code before sending patches.
>Thanks!

So sorry for that I built quickly using the wrong compiler and the wrong
target machine with my old .config that's horrible :)

Thanks a lot,
Karim