firejail.h: add missing linux/limits.h include & include cleanup

[kmk3]

Should fix #4578.

Cc: @Duncaen @dm9pZCAq (from #4578)

[kmk3]

From https://github.com/netblue30/firejail/pull/4583/checks?check_run_id=3736745444:

make[1]: Entering directory '/home/runner/work/firejail/firejail/src/libtracelog'
clang-11 -ggdb -W -Wall -Werror -O2 -DVERSION='"0.9.67"' -fstack-protector-all -D_FORTIFY_SOURCE=2 -fPIC -Wformat -Wformat-security  -c libtracelog.c -o libtracelog.o
libtracelog.c:724:2: error: expected expression
        int rv = orig_fchdir(fd);
        ^
libtracelog.c:725:9: error: use of undeclared identifier 'rv'
        return rv;
               ^
2 errors generated.
make[1]: *** [Makefile:18: libtracelog.o] Error 1
make: *** [Makefile:42: src/libtracelog/libtracelog.so] Error 2
make[1]: Leaving directory '/home/runner/work/firejail/firejail/src/libtracelog'
Error: Process completed with exit code 2.

From https://github.com/netblue30/firejail/pull/4583/checks?check_run_id=3736745378:

  + make
  libtracelog.c: In function ‘fchdir’:
  libtracelog.c:724:2: error: a label can only be part of a statement and a declaration is not a statement
    724 |  int rv = orig_fchdir(fd);
        |  ^~~
  make[1]: *** [Makefile:18: libtracelog.o] Error 1
  make: *** [Makefile:42: src/libtracelog/libtracelog.so] Error 2

Interesting, this compiles with CC=gcc but fails with CC=clang.

Fixed by replacing ret: with ret:; and force-pushed.

[rusty-snake]

OT: +1 for leaving a comment about force push changes.

[kmk3]

@dm9pZCAq Could you test if this branch fixes #4578?

[Duncaen]

limits.h should be fine, simple grep does not work because it includes other files, but its the right header for PATH_MAX.

[dm9pZCAq]

@dm9pZCAq Could you test if this branch fixes?

yes, everything compiles successfully without any warnings or errors

thank you

[kmk3]

Force-push changes:

Dropped these 2 commits:

• f4171df ("Fix wrong limits.h include (should be linux/limits.h)")
• 4ba2b17 ("Get ARG_MAX/PATH_MAX from sysconf/pathconf instead of linux/limits.h")

They require some changes and I found an alternative solution in one file. I
might submit them separately later.

Reworded this commit:

• e228db3 ("firejail.h: add missing linux/limits.h include")

And added a comment about ARG_MAX.

---

@Duncaen commented on Sep 28:

limits.h should be fine, simple grep does not work because it includes
other files, but its the right header for PATH_MAX.

Using limits.h breaks the build on Artix. After much debugging and confusion,
the answer was in the POSIX man pages all along. I'll just copy and paste the
reworded commit (579f856):

firejail.h: add missing linux/limits.h include

firejail.h uses PATH_MAX when defining a macro. Note that ARG_MAX and
PATH_MAX are not guaranteed to be (and potentially should not be)
defined. From POSIX.1-2017's limits.h(0p)[1]:

A definition of one of the symbolic constants in the following list
shall be omitted from the <limits.h> header on specific
implementations where the corresponding value is equal to or greater
than the stated minimum, but where the value can vary depending on the
file to which it is applied. The actual value supported for a
specific pathname shall be provided by the pathconf() function.

Use linux/limits.h instead of limits.h because glibc's limits.h
deliberately undefines ARG_MAX. See glibc commit f96853beaf
("* sysdeps/unix/sysv/linux/bits/local_lim.h: Undefined ARG_MAX if",
2008-03-27)[2].

From /usr/include/bits/local_lim.h (glibc 2.33-5 on Artix Linux):

#ifndef ARG_MAX
# define __undef_ARG_MAX
#endif

/* The kernel sources contain a file with all the needed information.  */
#include <linux/limits.h>
/* [...] */
/* Have to remove ARG_MAX?  */
#ifdef __undef_ARG_MAX
# undef ARG_MAX
# undef __undef_ARG_MAX
#endif

So if a file uses ARG_MAX (currently only cmdline.c) and limits.h (or a
firejail.h that includes limits.h) is included before linux/limits.h,
then the build will fail on glibc. Build log from using limits.h
(instead of linux/limits.h) on firejail.h:

$ make clean >/dev/null && make >/dev/null
cmdline.c:145:12: error: use of undeclared identifier 'ARG_MAX'; did you mean 'CFG_MAX'?
        if (len > ARG_MAX) {
                  ^~~~~~~
                  CFG_MAX
./firejail.h:805:2: note: 'CFG_MAX' declared here
        CFG_MAX // this should always be the last entry
        ^
[...]

Fixes #4578.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html
[2] https://sourceware.org/git/?p=glibc.git;a=commit;h=f96853beafc26d4f030961b0b67a79b5bfad5733

[kmk3]

@dm9pZCAq commented on Sep 28:

@dm9pZCAq Could you test if this branch fixes?

yes, everything compiles successfully without any warnings or errors

thank you

Thanks for testing.

[netblue30]

I ended up merging it myself!