uapi: MAX_ADDR_LEN vs. numeric 32

Hi,

First, thanks Dmitry for fixing several uapi compilation problems in
user space. I got a bit demotivated about the slow review progress, e.g.
no feedback what so ever, on some of the patches, but lets try again...

I rebased my tree now and saw

commit 745cb7f8a5de0805cade3de3991b7a95317c7c73
Author: Dmitry V. Levin <ldv@altlinux.org>
Date:   Tue Mar 7 23:50:50 2017 +0300

    uapi: fix linux/packet_diag.h userspace compilation error

which does:


since netdevice.h has the definition also in user space

#define MAX_ADDR_LEN    32              /* Largest hardware address length */

I find using MAX_ADDR_LEN better than numeric 32, though I doubt this will
change any time soon. Would you mind if I change packet_diag.h and
if_link.h to use that instead and fix the userspace compilation
problems by including netdevice.h?

Thanks,

-Mikko

Hi,

On Sat, Aug 05, 2017 at 12:33:25AM +0300, Mikko Rapeli wrote:
> First, thanks Dmitry for fixing several uapi compilation problems in
> user space. I got a bit demotivated

That's quite understandable.

> about the slow review progress, e.g.
> no feedback what so ever, on some of the patches, but lets try again...
> 
> I rebased my tree now and saw
> 
> commit 745cb7f8a5de0805cade3de3991b7a95317c7c73
> Author: Dmitry V. Levin <ldv@altlinux.org>
> Date:   Tue Mar 7 23:50:50 2017 +0300
> 
>     uapi: fix linux/packet_diag.h userspace compilation error
> 
> which does:
> 
> --- a/include/uapi/linux/packet_diag.h
> +++ b/include/uapi/linux/packet_diag.h
> @@ -64,7 +64,7 @@ struct packet_diag_mclist {
>         __u32   pdmc_count;
>         __u16   pdmc_type;
>         __u16   pdmc_alen;
> -       __u8    pdmc_addr[MAX_ADDR_LEN];
> +       __u8    pdmc_addr[32]; /* MAX_ADDR_LEN */
>  };
>  
>  struct packet_diag_ring {
> 
> In my tree I had fixed that case with:
> 
> --- a/include/uapi/linux/packet_diag.h
> +++ b/include/uapi/linux/packet_diag.h
> @@ -2,6 +2,7 @@
>  #define __PACKET_DIAG_H__
>  
>  #include <linux/types.h>
> +#include <linux/netdevice.h>
>  
>  struct packet_diag_req {
>         __u8    sdiag_family;
> 
> since netdevice.h has the definition also in user space
> 
> #define MAX_ADDR_LEN    32              /* Largest hardware address length */
> 
> I find using MAX_ADDR_LEN better than numeric 32, though I doubt this will
> change any time soon. Would you mind if I change packet_diag.h and
> if_link.h to use that instead and fix the userspace compilation
> problems by including netdevice.h?

The alternative fix, that is, to include <linux/netdevice.h>
which pulls in other headers and a lot of definitions with them,
has been mentioned in the discussion, too.
We decided that the fix that was applied would be the least of all evils.

On Sat, Aug 05, 2017 at 01:25:19AM +0300, Dmitry V. Levin wrote:
>
> On Sat, Aug 05, 2017 at 12:33:25AM +0300, Mikko Rapeli wrote:
> > 
> > I find using MAX_ADDR_LEN better than numeric 32, though I doubt this will
> > change any time soon. Would you mind if I change packet_diag.h and
> > if_link.h to use that instead and fix the userspace compilation
> > problems by including netdevice.h?
> 
> The alternative fix, that is, to include <linux/netdevice.h>
> which pulls in other headers and a lot of definitions with them,
> has been mentioned in the discussion, too.
> We decided that the fix that was applied would be the least of all evils.

Ok, that's fine then. I'll drop my netdevice.h inclusion patch.

Thanks,

-Mikko