Re: [PATCH 2/2] netlink: Diag core and basic socket info dumping

On Thu, Mar 21, 2013 at 12:52:30PM +0000, Thomas Graf wrote:

On 03/21/13 at 01:21pm, Andrey Vagin wrote:

quoted

diff --git a/include/uapi/linux/netlink_diag.h b/include/uapi/linux/netlink_diag.h
new file mode 100644
index 0000000..9328866
--- /dev/null
+++ b/include/uapi/linux/netlink_diag.h
+enum {
+	NETLINK_DIAG_MEMINFO,
+	NETLINK_DIAG_GROUPS,
+
+	NETLINK_DIAG_MAX,
+};

Please follow the common pattern and define NETLINK_DIAG_MAX as
NETLINK_DIAG_GROUPS like other by doing>

	[...] 
        __NETLINK_DIAG_MAX,
};
 
#define NETLINK_DIAG_MAX (__NETLINK_DIAG_MAX - 1)
 
Everyone is used to do:
 
        struct nlattr *attrs[NETLINK_DIAG_MAX+1];
         
        nla_parse([...], NETLINK_DIAG_MAX, [...]

Thank you for this issue. I sent the separate patch
"[PATCH] net: fix *_DIAG_MAX constants", because currently only
INET_DIAG_MAX is correct.

In fact, the follow-up patch to ss is buggy because of this.
UNIX_DIAG_MAX suffers from the same problem which is problem the
cause for this.

The code in ss looks like you described:
        struct rtattr *tb[UNIX_DIAG_MAX+1];
...
        parse_rtattr(tb, UNIX_DIAG_MAX, (struct rtattr*)(r+1),
                     nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));


        struct rtattr *tb[NETLINK_DIAG_MAX+1];
...
        parse_rtattr(tb, NETLINK_DIAG_MAX, (struct rtattr*)(r+1),
                     nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)))

I think I should only update headers... Or I don't understand something.