2013-03-21 14:21:35

by Andrei Vagin

[permalink] [raw]
Subject: [PATCH] net: fix *_DIAG_MAX constants

Follow the common pattern and define *_DIAG_MAX like:

[...]
__XXX_DIAG_MAX,
};

Because everyone is used to do:

struct nlattr *attrs[XXX_DIAG_MAX+1];

nla_parse([...], XXX_DIAG_MAX, [...]

Reported-by: Thomas Graf <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Pavel Emelyanov <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: David Howells <[email protected]>
Signed-off-by: Andrey Vagin <[email protected]>
---
include/uapi/linux/netlink_diag.h | 4 +++-
include/uapi/linux/packet_diag.h | 4 +++-
include/uapi/linux/unix_diag.h | 4 +++-
3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/netlink_diag.h b/include/uapi/linux/netlink_diag.h
index 9328866..88009a3 100644
--- a/include/uapi/linux/netlink_diag.h
+++ b/include/uapi/linux/netlink_diag.h
@@ -29,9 +29,11 @@ enum {
NETLINK_DIAG_MEMINFO,
NETLINK_DIAG_GROUPS,

- NETLINK_DIAG_MAX,
+ __NETLINK_DIAG_MAX,
};

+#define NETLINK_DIAG_MAX (__NETLINK_DIAG_MAX - 1)
+
#define NDIAG_PROTO_ALL ((__u8) ~0)

#define NDIAG_SHOW_MEMINFO 0x00000001 /* show memory info of a socket */
diff --git a/include/uapi/linux/packet_diag.h b/include/uapi/linux/packet_diag.h
index 93f5fa9..afafd70 100644
--- a/include/uapi/linux/packet_diag.h
+++ b/include/uapi/linux/packet_diag.h
@@ -33,9 +33,11 @@ enum {
PACKET_DIAG_TX_RING,
PACKET_DIAG_FANOUT,

- PACKET_DIAG_MAX,
+ __PACKET_DIAG_MAX,
};

+#define PACKET_DIAG_MAX (__PACKET_DIAG_MAX - 1)
+
struct packet_diag_info {
__u32 pdi_index;
__u32 pdi_version;
diff --git a/include/uapi/linux/unix_diag.h b/include/uapi/linux/unix_diag.h
index b8a2494..b9e2a6a 100644
--- a/include/uapi/linux/unix_diag.h
+++ b/include/uapi/linux/unix_diag.h
@@ -39,9 +39,11 @@ enum {
UNIX_DIAG_MEMINFO,
UNIX_DIAG_SHUTDOWN,

- UNIX_DIAG_MAX,
+ __UNIX_DIAG_MAX,
};

+#define UNIX_DIAG_MAX (__UNIX_DIAG_MAX - 1)
+
struct unix_diag_vfs {
__u32 udiag_vfs_ino;
__u32 udiag_vfs_dev;
--
1.8.1.4


2013-03-21 14:42:23

by Thomas Graf

[permalink] [raw]
Subject: Re: [PATCH] net: fix *_DIAG_MAX constants

On 03/21/13 at 06:18pm, Andrey Vagin wrote:
> Follow the common pattern and define *_DIAG_MAX like:
>
> [...]
> __XXX_DIAG_MAX,
> };
>
> Because everyone is used to do:
>
> struct nlattr *attrs[XXX_DIAG_MAX+1];
>
> nla_parse([...], XXX_DIAG_MAX, [...]
>
> Reported-by: Thomas Graf <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Pavel Emelyanov <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: "Paul E. McKenney" <[email protected]>
> Cc: David Howells <[email protected]>
> Signed-off-by: Andrey Vagin <[email protected]>

Acked-by: Thomas Graf <[email protected]>

2013-03-21 15:14:41

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] net: fix *_DIAG_MAX constants

From: Thomas Graf <[email protected]>
Date: Thu, 21 Mar 2013 14:42:18 +0000

> On 03/21/13 at 06:18pm, Andrey Vagin wrote:
>> Follow the common pattern and define *_DIAG_MAX like:
>>
>> [...]
>> __XXX_DIAG_MAX,
>> };
>>
>> Because everyone is used to do:
>>
>> struct nlattr *attrs[XXX_DIAG_MAX+1];
>>
>> nla_parse([...], XXX_DIAG_MAX, [...]
>>
>> Reported-by: Thomas Graf <[email protected]>
>> Cc: "David S. Miller" <[email protected]>
>> Cc: Pavel Emelyanov <[email protected]>
>> Cc: Eric Dumazet <[email protected]>
>> Cc: "Paul E. McKenney" <[email protected]>
>> Cc: David Howells <[email protected]>
>> Signed-off-by: Andrey Vagin <[email protected]>
>
> Acked-by: Thomas Graf <[email protected]>

So you're ACK'ing a patch that makes changes to files that don't even
exist in the repository?

Andrey, post a clean patch against 'net' that fixes these constants
for existing code, don't just assume that your original patch set is
applied and post changes relative to that. That's not how we work.

After the bug fix for the existing cases goes in, you have to repost
your original patch set on top of that.

2013-03-21 15:25:26

by Thomas Graf

[permalink] [raw]
Subject: Re: [PATCH] net: fix *_DIAG_MAX constants

On 03/21/13 at 11:14am, David Miller wrote:
> So you're ACK'ing a patch that makes changes to files that don't even
> exist in the repository?

I have been ACK'ing the patch in the context of the previous
patch that I reviewed in the first place which in summary is
now OK. But you are obviously right that a fixed version of
the initial patch should be submitted instead.