Turn some -Wunused-* diagnostics into errors, and solve them #1394

alejandro-colomar · 2025-11-29T23:30:53Z

This is a subset of -Werror=... #1220 at v24b.
Queued after Use const correctly. #1393.

Revisions:

v2

Add a few more MAYBE_UNUSED in code touched by recent patches.

$ git range-diff gh/const gh/unused unused 
1:  544f2e24f = 1:  544f2e24f src/: Unname unused parameter of main()
2:  1614d6e63 = 2:  1614d6e63 lib/, src/ tests/: Unname unused parameters in callbacks
3:  ea51596be = 3:  ea51596be lib/logind.c: Unname unused function parameter
4:  5b4e87cf1 ! 4:  5ac58a0aa lib/, src/: Add [[gnu::unused]] to parameters used in conditionally-compiled code
    @@ lib/log.c: void dolastlog (
        int fd;
        off_t offset;
     
    + ## src/groupmems.c ##
    +@@
    + #include <pwd.h>
    + 
    + #include "alloc/malloc.h"
    ++#include "attr.h"
    + #include "defines.h"
    + #include "groupio.h"
    + #include "prototypes.h"
    +@@ src/groupmems.c: static void process_flags (int argc, char **argv, struct option_flags *flags)
    + 
    + }
    + 
    +-static void check_perms (bool process_selinux)
    ++static void
    ++check_perms(MAYBE_UNUSED bool process_selinux)
    + {
    +   if (!list) {
    + #ifdef USE_PAM
    +
      ## src/newusers.c ##
     @@
      #include "alloc/reallocf.h"
    @@ src/newusers.c: static int update_passwd (struct passwd *pwd, const char *passwo
      {
        const struct spwd *sp;
        struct spwd spent;
    +@@ src/newusers.c: static void check_flags (void)
    +  *
    +  *        It will not return if the user is not allowed.
    +  */
    +-static void check_perms (struct option_flags *flags)
    ++static void
    ++check_perms(MAYBE_UNUSED struct option_flags *flags)
    + {
    + #ifdef ACCT_TOOLS_SETUID
    + #ifdef USE_PAM
5:  730df70cb = 5:  cbe8e96d9 lib/prefix_flag.c: Add [[gnu::unused]] to variable used in conditionally-compiled code
6:  fccf985fa = 6:  3a75e29a1 autogen.sh: CFLAGS: Promose some -Wunused-* to an error

v2b

Add some more.

$ git range-diff gh/const gh/unused unused
1:  544f2e24f = 1:  544f2e24f src/: Unname unused parameter of main()
2:  1614d6e63 = 2:  1614d6e63 lib/, src/ tests/: Unname unused parameters in callbacks
3:  ea51596be = 3:  ea51596be lib/logind.c: Unname unused function parameter
4:  5ac58a0aa ! 4:  d6490c042 lib/, src/: Add [[gnu::unused]] to parameters used in conditionally-compiled code
    @@ Commit message
     
         Signed-off-by: Alejandro Colomar <[email protected]>
     
    + ## lib/commonio.c ##
    +@@ lib/commonio.c: static int write_all (const struct commonio_db *db)
    + }
    + 
    + 
    +-int commonio_close (struct commonio_db *db, bool process_selinux)
    ++int
    ++commonio_close(struct commonio_db *db, MAYBE_UNUSED bool process_selinux)
    + {
    +   bool         errors = false;
    +   char         buf[1024];
    +
      ## lib/copydir.c ##
     @@ lib/copydir.c: static int copy_hardlink (const struct path_info *dst,
       *        Return 0 on success, -1 on error.
5:  cbe8e96d9 = 5:  d44afef05 lib/prefix_flag.c: Add [[gnu::unused]] to variable used in conditionally-compiled code
6:  3a75e29a1 = 6:  b6333f725 autogen.sh: CFLAGS: Promose some -Wunused-* to an error

v3

Cherry-pick a commit from -Werror=... #1220 v25 to fix a diagnostic.

$ git rd 
 1:  21105c175 =  1:  21105c175 lib/string/strspn/: Add missing const
 2:  1079f1921 =  2:  1079f1921 autogen.sh: CFLAGS: Promote -Wdiscarded-qualifiers to an error
 3:  544f2e24f =  3:  544f2e24f src/: Unname unused parameter of main()
 4:  1614d6e63 =  4:  1614d6e63 lib/, src/ tests/: Unname unused parameters in callbacks
 5:  ea51596be =  5:  ea51596be lib/logind.c: Unname unused function parameter
 6:  d6490c042 =  6:  d6490c042 lib/, src/: Add [[gnu::unused]] to parameters used in conditionally-compiled code
 7:  d44afef05 =  7:  d44afef05 lib/prefix_flag.c: Add [[gnu::unused]] to variable used in conditionally-compiled code
 8:  b6333f725 =  8:  b6333f725 autogen.sh: CFLAGS: Promose some -Wunused-* to an error
 -:  --------- >  9:  00303a2d8 lib/sssd.h: sssd_flush_cache(): Define as static inline function

v3b

Rebase

$ git rd 
 1:  21105c175 <  -:  --------- lib/string/strspn/: Add missing const
 2:  1079f1921 <  -:  --------- autogen.sh: CFLAGS: Promote -Wdiscarded-qualifiers to an error
 3:  544f2e24f =  1:  5ec29f580 src/: Unname unused parameter of main()
 4:  1614d6e63 =  2:  6c7f051e3 lib/, src/ tests/: Unname unused parameters in callbacks
 5:  ea51596be =  3:  affb78c15 lib/logind.c: Unname unused function parameter
 6:  d6490c042 =  4:  8b7532621 lib/, src/: Add [[gnu::unused]] to parameters used in conditionally-compiled code
 7:  d44afef05 =  5:  1105aaef9 lib/prefix_flag.c: Add [[gnu::unused]] to variable used in conditionally-compiled code
 8:  b6333f725 =  6:  324264719 autogen.sh: CFLAGS: Promose some -Wunused-* to an error
 9:  00303a2d8 =  7:  1ce93180e lib/sssd.h: sssd_flush_cache(): Define as static inline function

v3c

Rebase
Add reviewed-by @hallyn, including some notes about a partial objection.

$ git rd 
1:  5ec29f580 ! 1:  7829ab840 src/: Unname unused parameter of main()
    @@ Metadata
      ## Commit message ##
         src/: Unname unused parameter of main()
     
    +    Reviewed-by: Serge Hallyn <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## src/grpconv.c ##
2:  6c7f051e3 ! 2:  decfeeb28 lib/, src/ tests/: Unname unused parameters in callbacks
    @@ Metadata
      ## Commit message ##
         lib/, src/ tests/: Unname unused parameters in callbacks
     
    +    [Serge: preference to keep the names as a comment, but ok]
    +    Reviewed-by: Serge Hallyn <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## lib/cleanup_user.c ##
3:  affb78c15 ! 3:  87a54b503 lib/logind.c: Unname unused function parameter
    @@ Metadata
      ## Commit message ##
         lib/logind.c: Unname unused function parameter
     
    +    [Serge: preference to keep the names as a comment, but ok]
    +    Reviewed-by: Serge Hallyn <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## lib/logind.c ##
4:  8b7532621 ! 4:  ee60b0b6e lib/, src/: Add [[gnu::unused]] to parameters used in conditionally-compiled code
    @@ Metadata
      ## Commit message ##
         lib/, src/: Add [[gnu::unused]] to parameters used in conditionally-compiled code
     
    +    Reviewed-by: Serge Hallyn <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## lib/commonio.c ##
    @@ src/newusers.c: static void check_flags (void)
       *
       *        It will not return if the user is not allowed.
       */
    --static void check_perms (struct option_flags *flags)
    +-static void check_perms(const struct option_flags *flags)
     +static void
    -+check_perms(MAYBE_UNUSED struct option_flags *flags)
    ++check_perms(MAYBE_UNUSED const struct option_flags *flags)
      {
      #ifdef ACCT_TOOLS_SETUID
      #ifdef USE_PAM
5:  1105aaef9 ! 5:  6f1fb7c97 lib/prefix_flag.c: Add [[gnu::unused]] to variable used in conditionally-compiled code
    @@ Metadata
      ## Commit message ##
         lib/prefix_flag.c: Add [[gnu::unused]] to variable used in conditionally-compiled code
     
    +    Reviewed-by: Serge Hallyn <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## lib/prefix_flag.c ##
6:  324264719 ! 6:  a413c7dc8 autogen.sh: CFLAGS: Promose some -Wunused-* to an error
    @@ Metadata
      ## Commit message ##
         autogen.sh: CFLAGS: Promose some -Wunused-* to an error
     
    +    Reviewed-by: Serge Hallyn <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## autogen.sh ##
7:  1ce93180e ! 7:  7c8c2b8cc lib/sssd.h: sssd_flush_cache(): Define as static inline function
    @@ Commit message
         below for that.
     
         Link: <https://github.com/shadow-maint/shadow/issues/1221>
    +    Reviewed-by: Serge Hallyn <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## lib/sssd.h ##

v4

Use (void)0 within _Generic(3) to avoid -Wunused-value diagnostics.

$ git rd 
1:  7829ab840 = 1:  7829ab840 src/: Unname unused parameter of main()
2:  decfeeb28 = 2:  decfeeb28 lib/, src/ tests/: Unname unused parameters in callbacks
3:  87a54b503 = 3:  87a54b503 lib/logind.c: Unname unused function parameter
4:  ee60b0b6e = 4:  ee60b0b6e lib/, src/: Add [[gnu::unused]] to parameters used in conditionally-compiled code
5:  6f1fb7c97 = 5:  6f1fb7c97 lib/prefix_flag.c: Add [[gnu::unused]] to variable used in conditionally-compiled code
6:  a413c7dc8 = 6:  a413c7dc8 autogen.sh: CFLAGS: Promose some -Wunused-* to an error
7:  7c8c2b8cc = 7:  7c8c2b8cc lib/sssd.h: sssd_flush_cache(): Define as static inline function
-:  --------- > 8:  f538994d9 lib/search/: Use (void)0 within _Generic(3) to avoid -Wunused-value diagnostics

hallyn · 2025-12-05T14:00:48Z

This is good stuff, but I would really like to be able to see straight
from the relevant code what this unnamed argument was supposed to be.
No big deal for int main, as we all know it's arguments, but for something
like

129  static int ni_conv (int num_msg,
130                      const struct pam_message **msg,
131                      struct pam_response **resp,
132 -                    MAYBE_UNUSED void *appdata_ptr)
133 +                    void *)
134  {

it's horrible to just see void * there.

What can we do to improve that? I'd be ok with just having

132 -                    void * /* appdata_ptr */)

or implementing an 'always_unused', like

132 -                    void * UNUSED appdata_ptr )

Or even just putting a comment above the function.

Yes, I could look it up, but that's an expensive context switch
while reading the code.

alejandro-colomar · 2025-12-05T14:21:03Z

This is good stuff, but I would really like to be able to see straight from the relevant code what this unnamed argument was supposed to be. No big deal for int main, as we all know it's arguments, but for something like
129  static int ni_conv (int num_msg,
130                      const struct pam_message **msg,
131                      struct pam_response **resp,
132 -                    MAYBE_UNUSED void *appdata_ptr)
133 +                    void *)
134  {
it's horrible to just see void * there.

What can we do to improve that? I'd be ok with just having
132 -                    void * /* appdata_ptr */)
or implementing an 'always_unused', like
132 -                    void * UNUSED appdata_ptr )
Or even just putting a comment above the function.

Yes, I could look it up, but that's an expensive context switch while reading the code.

I think we shouldn't care what that unnamed argument is. We should think of it as "something that needs to be there because some function pointer demands it but we don't care about it at all".

hallyn · 2025-12-05T14:27:04Z

I think we shouldn't care what that unnamed argument is. We should think of it as "something that needs to be there because some function pointer demands it but we don't care about it at all".

But that's not how the inquisitive human mind works. It'll be
distracting.

Anyway, I'll merge the pr, but am lodging my objection to that bit :)

Thanks.

alejandro-colomar · 2025-12-05T14:37:50Z

I think we shouldn't care what that unnamed argument is. We should think of it as "something that needs to be there because some function pointer demands it but we don't care about it at all".

But that's not how the inquisitive human mind works. It'll be distracting.

Anyway, I'll merge the pr, but am lodging my objection to that bit :)

Hmmm, I'll keep that partial objection in mind for the future, in case I have any better ideas. :)

Thank you!

Thanks.

hallyn · 2025-12-05T14:42:36Z

(I'm convinced you're working on it, but just to make sure, I was waiting
on you to push the rebase. If you prefer, I'm happy to rebase myself and then
merge)

alejandro-colomar · 2025-12-05T14:44:42Z

(I'm convinced you're working on it, but just to make sure, I was waiting on you to push the rebase. If you prefer, I'm happy to rebase myself and then merge)

Yup, I'm working on the rebase. :-)

Reviewed-by: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

[Serge: preference to keep the names as a comment, but ok] Reviewed-by: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

…ompiled code Reviewed-by: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

…lly-compiled code Reviewed-by: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

Reviewed-by: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

Being a macro, the unused return value triggers a diagnostic. This probably needs further investigation, but we have the issue linked below for that. Link: <shadow-maint#1221> Reviewed-by: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

alejandro-colomar · 2025-12-05T14:50:52Z

Done with the rebase, @hallyn. I included Reviewed-by's for you, and some notes about your preference.

…iagnostics Signed-off-by: Alejandro Colomar <[email protected]>

hallyn · 2025-12-05T15:22:01Z

Thanks

alejandro-colomar requested review from hallyn and ikerexxe November 29, 2025 23:30

alejandro-colomar force-pushed the unused branch from fccf985 to 3a75e29 Compare November 29, 2025 23:37

alejandro-colomar mentioned this pull request Nov 29, 2025

-Werror=... #1220

Draft

alejandro-colomar force-pushed the unused branch from 3a75e29 to b6333f7 Compare November 29, 2025 23:54

alejandro-colomar changed the title ~~Turn some -Wunused-* diagnostic into errors, and solve them~~ Turn some -Wunused-* diagnostics into errors, and solve them Nov 30, 2025

alejandro-colomar force-pushed the unused branch from 00303a2 to 1ce9318 Compare December 5, 2025 10:13

alejandro-colomar marked this pull request as ready for review December 5, 2025 10:13

alejandro-colomar mentioned this pull request Dec 5, 2025

4.19 Release plan (Herve) - 2025-12 #1354

Closed

alejandro-colomar added 7 commits December 5, 2025 15:45

src/: Unname unused parameter of main()

7829ab8

Reviewed-by: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

lib/, src/ tests/: Unname unused parameters in callbacks

decfeeb

[Serge: preference to keep the names as a comment, but ok] Reviewed-by: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

lib/logind.c: Unname unused function parameter

87a54b5

[Serge: preference to keep the names as a comment, but ok] Reviewed-by: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

lib/, src/: Add [[gnu::unused]] to parameters used in conditionally-c…

ee60b0b

…ompiled code Reviewed-by: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

lib/prefix_flag.c: Add [[gnu::unused]] to variable used in conditiona…

6f1fb7c

…lly-compiled code Reviewed-by: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

autogen.sh: CFLAGS: Promose some -Wunused-* to an error

a413c7d

Reviewed-by: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

alejandro-colomar force-pushed the unused branch from 1ce9318 to 7c8c2b8 Compare December 5, 2025 14:49

lib/search/: Use (void)0 within _Generic(3) to avoid -Wunused-value d…

f538994

…iagnostics Signed-off-by: Alejandro Colomar <[email protected]>

hallyn merged commit 8a5d35c into shadow-maint:master Dec 5, 2025
11 checks passed

alejandro-colomar deleted the unused branch December 5, 2025 19:30

alejandro-colomar self-assigned this Dec 27, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Turn some -Wunused-* diagnostics into errors, and solve them #1394

Turn some -Wunused-* diagnostics into errors, and solve them #1394

Uh oh!

alejandro-colomar commented Nov 29, 2025 •

edited

Loading

Uh oh!

hallyn commented Dec 5, 2025

Uh oh!

alejandro-colomar commented Dec 5, 2025

Uh oh!

hallyn commented Dec 5, 2025

Uh oh!

alejandro-colomar commented Dec 5, 2025

Uh oh!

hallyn commented Dec 5, 2025

Uh oh!

alejandro-colomar commented Dec 5, 2025

Uh oh!

alejandro-colomar commented Dec 5, 2025

Uh oh!

Uh oh!

hallyn commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Turn some -Wunused-* diagnostics into errors, and solve them #1394

Turn some -Wunused-* diagnostics into errors, and solve them #1394

Uh oh!

Conversation

alejandro-colomar commented Nov 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hallyn commented Dec 5, 2025

Uh oh!

alejandro-colomar commented Dec 5, 2025

Uh oh!

hallyn commented Dec 5, 2025

Uh oh!

alejandro-colomar commented Dec 5, 2025

Uh oh!

hallyn commented Dec 5, 2025

Uh oh!

alejandro-colomar commented Dec 5, 2025

Uh oh!

alejandro-colomar commented Dec 5, 2025

Uh oh!

Uh oh!

hallyn commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alejandro-colomar commented Nov 29, 2025 •

edited

Loading