Skip to content

Conversation

@alejandro-colomar
Copy link
Collaborator

@alejandro-colomar alejandro-colomar commented Oct 19, 2025

@ikerexxe , as you said you like this more than the addition of eprintf(), I've split these two patches from the other larger PR, so that this can be merged earlier.


Revisions:

v1b
  • Rebase
$ git rd 
1:  093ab3f0 = 1:  6a5f2898 lib/getdef.h: Add missing includes
2:  2f4eec2b ! 2:  c099a5f9 lib/string/: strerrno(): Add function
    @@ Commit message
     
      ## lib/Makefile.am ##
     @@ lib/Makefile.am: libshadow_la_SOURCES = \
    -   string/strdup/xstrdup.h \
    -   string/strdup/xstrndup.c \
    -   string/strdup/xstrndup.h \
    +   string/strdup/strndupa.h \
    +   string/strdup/strndup.c \
    +   string/strdup/strndup.h \
     +  string/strerrno.c \
     +  string/strerrno.h \
        string/strftime.c \
3:  ad1c40cf ! 3:  3519a8c7 lib/, src/: Use strerrno() instead of its pattern
    @@ lib/addgrps.c: add_groups(const char *list)
        }
      
     
    - ## lib/alloc/x/xcalloc.c ##
    -@@
    - 
    - #include "defines.h"
    - #include "shadowlog.h"
    -+#include "string/strerrno.h"
    - 
    - 
    - void *
    -@@ lib/alloc/x/xcalloc.c: xcalloc(size_t nmemb, size_t size)
    -   return p;
    - 
    - x:
    --  fprintf(log_get_logfd(), _("%s: %s\n"),
    --          log_get_progname(), strerror(errno));
    -+  fprintf(log_get_logfd(), _("%s: %s\n"), log_get_progname(), strerrno());
    -   exit(13);
    - }
    -
    - ## lib/alloc/x/xrealloc.c ##
    -@@
    - #include "alloc/reallocf.h"
    - #include "defines.h"
    - #include "shadowlog.h"
    -+#include "string/strerrno.h"
    - 
    - 
    - void *
    -@@ lib/alloc/x/xrealloc.c: xreallocarray(void *p, size_t nmemb, size_t size)
    -   return p;
    - 
    - x:
    --  fprintf(log_get_logfd(), _("%s: %s\n"),
    --          log_get_progname(), strerror(errno));
    -+  fprintf(log_get_logfd(), _("%s: %s\n"), log_get_progname(), strerrno());
    -   exit(13);
    - }
    -
      ## lib/commonio.c ##
     @@
      #include "string/sprintf/snprintf.h"
    @@ lib/commonio.c: static int do_lock_file (const char *file, const char *lock, boo
     
      ## lib/copydir.c ##
     @@
    - #include "string/sprintf/xaprintf.h"
    + #include "string/sprintf/aprintf.h"
      #include "string/strcmp/streq.h"
      #include "string/strcmp/strprefix.h"
     +#include "string/strerrno.h"
    @@ lib/copydir.c: static void error_acl (MAYBE_UNUSED struct error_context *ctx, co
      }
      
     
    + ## lib/exit_if_null.h ##
    +@@
    + 
    + #include "config.h"
    + 
    +-#include <errno.h>
    + #include <stddef.h>
    + #include <stdio.h>
    + #include <stdlib.h>
    +-#include <string.h>
    + 
    + #include "shadowlog.h"
    ++#include "string/strerrno.h"
    + 
    + 
    + /*
    +@@ lib/exit_if_null.h: inline void
    + exit_if_null_(void *p)
    + {
    +   if (p == NULL) {
    +-          fprintf(log_get_logfd(), "%s: %s\n",
    +-                  log_get_progname(), strerror(errno));
    ++          fprintf(log_get_logfd(), "%s: %s\n", log_get_progname(), strerrno());
    +           exit(13);
    +   }
    + }
    +
      ## lib/find_new_gid.c ##
     @@
      #include "groupio.h"
    @@ lib/idmapping.c: void write_mapping(int proc_dir_fd, int ranges, const struct ma
     
      ## lib/prefix_flag.c ##
     @@
    - #include "string/sprintf/xaprintf.h"
    + #include "string/sprintf/aprintf.h"
      #include "string/strcmp/streq.h"
      #include "string/strcmp/strprefix.h"
     +#include "string/strerrno.h"
    @@ lib/spawn.c: run_command(const char *cmd, const char *argv[],
        }
      
     
    - ## lib/string/strtok/xastrsep2ls.h ##
    -@@
    - #include "attr.h"
    - #include "shadowlog.h"
    - #include "string/strtok/astrsep2ls.h"
    -+#include "string/strerrno.h"
    - 
    - 
    - ATTR_ACCESS(read_write, 1) ATTR_ACCESS(write_only, 3)
    -@@ lib/string/strtok/xastrsep2ls.h: xastrsep2ls(char *s, const char *restrict delim, size_t *restrict np)
    - 
    -   return ls;
    - x:
    --  fprintf(log_get_logfd(), "%s: %s\n",
    --          log_get_progname(), strerror(errno));
    -+  fprintf(log_get_logfd(), "%s: %s\n", log_get_progname(), strerrno());
    -   exit(13);
    - }
    - 
    -
      ## lib/tcbfuncs.c ##
     @@
      #include "shadowlog_internal.h"
    @@ src/chage.c
     @@
      #include "string/strcmp/streq.h"
      #include "string/strcpy/strtcpy.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strdup.h"
     +#include "string/strerrno.h"
      #include "string/strftime.h"
      #include "time/day_to_str.h"
    @@ src/gpasswd.c
     @@
      #include "string/strcmp/streq.h"
      #include "string/strcpy/strtcpy.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strdup.h"
     +#include "string/strerrno.h"
    ++
      
      struct option_flags {
        bool chroot;
    @@ src/login.c
     @@
      #include "string/strcmp/strprefix.h"
      #include "string/strcpy/strtcpy.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strdup.h"
     +#include "string/strerrno.h"
      #include "string/strftime.h"
      
    @@ src/newgrp.c
     @@
      #include "string/strcmp/streq.h"
      #include "string/strcmp/strprefix.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strdup.h"
     +#include "string/strerrno.h"
      
      #include <assert.h>
    @@ src/newusers.c
     @@
      #include "string/sprintf/snprintf.h"
      #include "string/strcmp/streq.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strdup.h"
     +#include "string/strerrno.h"
      #include "string/strtok/stpsep.h"
      #include "string/strtok/strsep2arr.h"
    @@ src/passwd.c
     @@
      #include "string/strcmp/strprefix.h"
      #include "string/strcpy/strtcpy.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strdup.h"
     +#include "string/strerrno.h"
      #include "time/day_to_str.h"
      
    @@ src/useradd.c
     @@
      #include "string/strcmp/streq.h"
      #include "string/strcmp/strprefix.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strdup.h"
     +#include "string/strerrno.h"
      #include "string/strtok/stpsep.h"
      
    @@ src/userdel.c
     @@
      #include "string/strcmp/streq.h"
      #include "string/strcmp/strprefix.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strdup.h"
     +#include "string/strerrno.h"
      
      
    @@ src/usermod.c
     @@
      #include "string/strcmp/streq.h"
      #include "string/strcmp/strprefix.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strdup.h"
     +#include "string/strerrno.h"
      #include "time/day_to_str.h"
      #include "typetraits.h"
    @@ src/usermod.c: static void update_faillog (void)
     
      ## src/vipw.c ##
     @@
    + #include "string/sprintf/aprintf.h"
      #include "string/sprintf/snprintf.h"
    - #include "string/sprintf/xaprintf.h"
      #include "string/strcmp/streq.h"
     +#include "string/strerrno.h"
      
v1c
  • Add dependency in test_xaprintf. [@ikerexxe ]
$ git rd 
1:  6a5f28988 = 1:  6a5f28988 lib/getdef.h: Add missing includes
2:  c099a5f99 = 2:  c099a5f99 lib/string/: strerrno(): Add function
3:  3519a8c7b ! 3:  415166cfd lib/, src/: Use strerrno() instead of its pattern
    @@ src/vipw.c: vipwedit (const char *file, int (*file_lock) (void), int (*file_unlo
      #ifdef WITH_TCB
                if (tcb_mode) {
                        free(to_rename);
    +
    + ## tests/unit/Makefile.am ##
    +@@ tests/unit/Makefile.am: test_xaprintf_SOURCES = \
    +     ../../lib/shadowlog.c \
    +     ../../lib/string/sprintf/aprintf.c \
    +     ../../lib/string/strcmp/streq.c \
    ++    ../../lib/string/strerrno.c \
    +     test_xaprintf.c \
    +     $(NULL)
    + test_xaprintf_CFLAGS = \
v1d
  • Improve comment.
$ git rd 
1:  6a5f28988 = 1:  6a5f28988 lib/getdef.h: Add missing includes
2:  c099a5f99 ! 2:  bc8a1c4f6 lib/string/: strerrno(): Add function
    @@ lib/string/strerrno.h (new)
     +inline const char *strerrno(void);
     +
     +
    -+// string errno
    ++// strerrno - string errno
     +inline const char *
     +strerrno(void)
     +{
3:  415166cfd = 3:  29b03b500 lib/, src/: Use strerrno() instead of its pattern
v2
  • Use a macro instead of an inline function. This makes sure this doesn't create hidden dependencies between source files. Now we can remove the dependency in the unit tests for xaprintf().

    I was using the inline function to const-ify the return value, as strerror(3) returns non-const (but can't be modified) which is dangerous. But we have const_cast(), which allows us to do the same in macros, so do that.

$ git rd 
1:  6a5f28988 = 1:  6a5f28988 lib/getdef.h: Add missing includes
2:  bc8a1c4f6 ! 2:  fbd2c3e37 lib/string/: strerrno(): Add function
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/string/: strerrno(): Add function
    +    lib/string/: strerrno(): Add macro
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    @@ lib/string/strerrno.c (new)
     +#include "config.h"
     +
     +#include "string/strerrno.h"
    -+
    -+
    -+extern inline const char *strerrno(void);
     
      ## lib/string/strerrno.h (new) ##
     @@
    @@ lib/string/strerrno.h (new)
     +#include <errno.h>
     +#include <string.h>
     +
    -+
    -+inline const char *strerrno(void);
    ++#include "cast.h"
     +
     +
     +// strerrno - string errno
    -+inline const char *
    -+strerrno(void)
    -+{
    -+  return strerror(errno);
    -+}
    ++#define strerrno()  const_cast(const char *, strerror(errno))
     +
     +
     +#endif  // include guard
3:  29b03b500 ! 3:  bd24414b5 lib/, src/: Use strerrno() instead of its pattern
    @@ src/vipw.c: vipwedit (const char *file, int (*file_lock) (void), int (*file_unlo
      #ifdef WITH_TCB
                if (tcb_mode) {
                        free(to_rename);
    -
    - ## tests/unit/Makefile.am ##
    -@@ tests/unit/Makefile.am: test_xaprintf_SOURCES = \
    -     ../../lib/shadowlog.c \
    -     ../../lib/string/sprintf/aprintf.c \
    -     ../../lib/string/strcmp/streq.c \
    -+    ../../lib/string/strerrno.c \
    -     test_xaprintf.c \
    -     $(NULL)
    - test_xaprintf_CFLAGS = \
v3
  • Use a compound literal for the const conversion. This is safer than a cast, as it only accepts implicit conversions.
$ git rd
1:  6a5f28988 = 1:  6a5f28988 lib/getdef.h: Add missing includes
2:  fbd2c3e37 ! 2:  d846bbd90 lib/string/: strerrno(): Add macro
    @@ lib/string/strerrno.h (new)
     +
     +
     +// strerrno - string errno
    -+#define strerrno()  const_cast(const char *, strerror(errno))
    ++#define strerrno()  ((const char *){strerror(errno)})
     +
     +
     +#endif  // include guard
3:  bd24414b5 = 3:  d2962006e lib/, src/: Use strerrno() instead of its pattern
v3b
  • Remove unused include.
$ git rd 
1:  6a5f28988 = 1:  6a5f28988 lib/getdef.h: Add missing includes
2:  d846bbd90 ! 2:  01065f109 lib/string/: strerrno(): Add macro
    @@ lib/string/strerrno.h (new)
     +#include <errno.h>
     +#include <string.h>
     +
    -+#include "cast.h"
    -+
     +
     +// strerrno - string errno
     +#define strerrno()  ((const char *){strerror(errno)})
3:  d2962006e = 3:  619a2ff74 lib/, src/: Use strerrno() instead of its pattern
v4
  • Replace a few more cases.
$ git rd 
1:  6a5f28988 = 1:  6a5f28988 lib/getdef.h: Add missing includes
2:  01065f109 = 2:  01065f109 lib/string/: strerrno(): Add macro
3:  619a2ff74 ! 3:  785ca7b62 lib/, src/: Use strerrno() instead of its pattern
    @@ src/useradd.c: set_defaults(void)
                        goto err_free_new;
                }
        }
    +@@ src/useradd.c: set_defaults(void)
    +   assert(SNPRINTF(buf, "%s-", default_file) != -1);
    +   unlink (buf);
    +   if ((link (default_file, buf) != 0) && (ENOENT != errno)) {
    +-          int err = errno;
    +           fprintf (stderr,
    +                    _("%s: Cannot create backup file (%s): %s\n"),
    +-                   Prog, buf, strerror (err));
    ++                   Prog, buf, strerrno());
    +           unlink (new_file);
    +           goto err_free_def;
    +   }
    +@@ src/useradd.c: set_defaults(void)
    +    * Rename the new default file to its correct name.
    +    */
    +   if (rename (new_file, default_file) != 0) {
    +-          int err = errno;
    +           fprintf (stderr,
    +                    _("%s: rename: %s: %s\n"),
    +-                   Prog, new_file, strerror (err));
    ++                   Prog, new_file, strerrno());
    +           goto err_free_def;
    +   }
    + #ifdef WITH_AUDIT
     @@ src/useradd.c: static void faillog_reset (uid_t uid)
        if (-1 == fd) {
                fprintf (stderr,

Signed-off-by: Alejandro Colomar <alx@kernel.org>
Copy link
Collaborator

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to update the unit test build configuration to include strerrno.c in the build for the test_xaprintf target, since that test uses exit_if_null.h which now depends on strerrno().

That should make CI green.

@alejandro-colomar
Copy link
Collaborator Author

You need to update the unit test build configuration to include strerrno.c in the build for the test_xaprintf target, since that test uses exit_if_null.h which now depends on strerrno().

That should make CI green.

Good eye. Thanks! :)

@alejandro-colomar alejandro-colomar force-pushed the strerrno branch 3 times, most recently from bd24414 to d296200 Compare November 3, 2025 11:14
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Collaborator Author

CI is passing now, @ikerexxe .

Signed-off-by: Alejandro Colomar <alx@kernel.org>
Copy link
Collaborator

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Merging.

@ikerexxe ikerexxe merged commit 90467ef into shadow-maint:master Nov 4, 2025
11 checks passed
@alejandro-colomar alejandro-colomar deleted the strerrno branch November 4, 2025 11:55
@alejandro-colomar alejandro-colomar self-assigned this Dec 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants