[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 <karim.eshapa@gmail.com>
---
 drivers/net/ethernet/adi/bfin_mac.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

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

Hi Karim,

On Sun, Apr 23, 2017 at 8:02 PM, Karim Eshapa <karim.eshapa@gmail.com> wrote:
> Use FIELD_SIZEOF defined kernel macro kernel.h
>
> Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
> ---
>  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 -- geert@linux-m68k.org

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

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