shadow-maint / shadow

Upstream shadow tree
Other
292 stars 228 forks source link

shadow 4.14.2: Build error 'parameter name omitted' in logind #918

Closed psaavedra closed 7 months ago

psaavedra commented 7 months ago

I'm getting this error building shadow 4.14.2:

/bin/sh ../libtool  --tag=CC   --mode=compile gcc   -I. -I../../../../../../../workspace/sources/shadow-native/lib -I..    -I../../../../../../../workspace/sources/shadow-native -isystem/tmp/work/x86_64-linux/shadow-native/4.14.2/recipe-sysroot-native/usr/include -DLIBBSD_OVERLAY -isystem /tmp/work/x86_64-linux/shadow-native/4.14.2/recipe-sysroot-native/usr/lib/pkgconfig/../../../usr/include/bsd  -lsystemd -isystem/tmp/work/x86_64-linux/shadow-native/4.14.2/recipe-sysroot-native/usr/include -O2 -pipe -MT libshadow_la-logind.lo -MD -MP -MF .deps/libshadow_la-logind.Tpo -c -o libshadow_la-logind.lo `test -f 'logind.c' || echo '../../../../../../../workspace/sources/shadow-native/lib/'`logind.c
libtool: compile:  gcc -I. -I../../../../../../../workspace/sources/shadow-native/lib -I.. -I../../../../../../../workspace/sources/shadow-native -isystem/tmp/work/x86_64-linux/shadow-native/4.14.2/recipe-sysroot-native/usr/include -DLIBBSD_OVERLAY -isystem /tmp/work/x86_64-linux/shadow-native/4.14.2/recipe-sysroot-native/usr/lib/pkgconfig/../../../usr/include/bsd -lsystemd -isystem/tmp/work/x86_64-linux/shadow-native/4.14.2/recipe-sysroot-native/usr/include -O2 -pipe -MT libshadow_la-logind.lo -MD -MP -MF .deps/libshadow_la-logind.Tpo -c ../../../../../../../workspace/sources/shadow-native/lib/logind.c  -fPIC -DPIC -o .libs/libshadow_la-logind.o
../../../../../../../workspace/sources/shadow-native/lib/logind.c: In function �~@~Xactive_sessions_count�~@~Y:
../../../../../../../workspace/sources/shadow-native/lib/logind.c:38:55: error: parameter name omitted
   38 | unsigned long active_sessions_count(const char *name, unsigned long unused)

I am using gcc (Debian 10.2.1-6) 10.2.1 20210110 in this context.

The following patch fix the error but I am not 100% sure if this is the exected:

diff --git a/lib/logind.c b/lib/logind.c
index d4d217c..da945d6 100644
--- a/lib/logind.c
+++ b/lib/logind.c
@@ -35,7 +35,7 @@ done:
     return ret;
 }

-unsigned long active_sessions_count(const char *name, unsigned long unused)
+unsigned long active_sessions_count(const char *name, unsigned long unused _)
 {
     struct passwd *pw;
     unsigned long count = 0;
alejandro-colomar commented 7 months ago

Your fix is correct; although _ is a name in use, by GNU gettext, as a function --or function-like macro; I don't know-- _()). You'll need to use a different name.

$ grepc unused .
./lib/attr.h:# define unused                      __attribute__((unused))
./lib/attr.h:# define unused
./src/su.c:static void kill_child (int unused(s));

unused is a macro defined by us. I happened to look at that code a few days ago, and was surprised that it didn't result in a build error, but since nobody reported such a problem, I guessed I was missing something. It seems I was right to suspect of it.

alejandro-colomar commented 7 months ago

I would also send a patch for renaming the unused macro to UNUSED. Would you mind?

psaavedra commented 7 months ago

I would also send a patch for renaming the unused macro to UNUSED. Would you mind?

Pushed a PR for master. I think another one is going to be needed for the 4.14.x branch.

jubalh commented 7 months ago

I think another one is going to be needed for the 4.14.x branch.

No PR needed. The commit will be picked.

psaavedra commented 7 months ago

No PR needed. The commit will be picked.

OK. The cherry-pick is not straightforward but easy to adapt. I left in my fork the backport commit just as reference: e1e95951b0b9553e801b57ccd153c42baa956caa

jubalh commented 7 months ago

What I meant to say is that after the PR to master you dont need to do anything. The maintainer of the stable branches will select the commits from master as they see fit.

alejandro-colomar commented 7 months ago

No PR needed. The commit will be picked.

OK. The cherry-pick is not straightforward but easy to adapt. I left in my fork the backport commit just as reference: e1e95951b0b9553e801b57ccd153c42baa956caa

Thanks, but for the stable branch, I'll omit the renaming of the macro, so the fix should be trivial to backport.

But yeah, thanks for suggesting a backport in case the cherry-pick gets tricky.