Re: Unused macro

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi,

On Sun, Apr 04, 2021 at 10:44:12PM +0200, Alejandro Colomar (man-pages) wrote:
> Hello Florian,
> 
> On 4/4/21 10:05 PM, Florian Westphal wrote:
> > Alejandro Colomar (man-pages) <alx.manpages@gmail.com> wrote:
> >> I was updating the includes on some manual pages, when I found that a
> >> macro used ARRAY_SIZE() without including a header that defines it.
> >> That surprised me, because it would more than likely result in a compile
> >> error, but of course, the macro wasn't being used:
> >>
> >> .../linux$ grep -rn SCTP_CHUNKMAP_IS_ALL_SET
> >> include/uapi/linux/netfilter/xt_sctp.h:80:#define
> >> SCTP_CHUNKMAP_IS_ALL_SET(chunkmap) \
> >> .../linux$
> > 
> > This is an UAPI header, this macro is used by userspace software, e.g.
> > iptables.
> > 
> 
> Ahh, I see.  Thanks.
> 
> Then we still have the issue that ARRAY_SIZE is not defined in that
> header (see a simple test below).  You should probably include some
> header that provides it.

SCTP_CHUNKMAP_IS_* macros are used from iptables/extensions/xt_sctp.h
(iptables userspace codebase). These macros are also used internally
from net/netfilter/xt_sctp.c. It's using a rather unorthodox trick to
share code between the kernel and userspace, otherwise iptables would
need to keep a copy of this code.

BTW, why do you need xt_sctp.h for the manpages? This header is rather
specific to the match on sctp from the xtables infrastructure, so it's
not so useful from a programmer perspective (manpages) I think.

> But again, if no one noticed this in more than a decade, either no one
> used this macro, or they included other headers in the same file where
> they used the macro.  So I'd still rethink if maybe that macro (and
> possibly others) is really needed.
> 
> Test 1:
> 
> [[
> $ cat test.c
> #include <linux/netfilter/xt_sctp.h>
> 
> int foo(int x)
> {
> 	int a[x];
> 
> 	return ARRAY_SIZE(a);
> }
> $ cc -Wall -Wextra -Werror test.c -S -o test.s
> test.c: In function ‘foo’:
> test.c:7:9: error: implicit declaration of function ‘ARRAY_SIZE’
> [-Werror=implicit-function-declaration]
>     7 |  return ARRAY_SIZE(a);
>       |         ^~~~~~~~~~
> cc1: all warnings being treated as errors
> $
> ]]

I see, this is breaking self-compilation of the headers.

If there a need to remove xt_sctp.h from the ignore-list of the header
self-compilation infrastructure, it should be possible to fix
userspace to keep its own copy and probably add a #warn on the UAPI
header to let other possible consumers of this macro that this macro
will go away at some point.