Skip to content

Add and use strnlen_a() instead of more complex code - #1298

Open
alejandro-colomar wants to merge 3 commits into
shadow-maint:masterfrom
alejandro-colomar:strnlen
Open

Add and use strnlen_a() instead of more complex code#1298
alejandro-colomar wants to merge 3 commits into
shadow-maint:masterfrom
alejandro-colomar:strnlen

Conversation

@alejandro-colomar

@alejandro-colomar alejandro-colomar commented Jul 16, 2025

Copy link
Copy Markdown
Collaborator

Cc: @uecker


Revisions:

v2
  • Simplify static-assertions by using SIZEOF_ARRAY.
$ git rd 
1:  2a4bb60f = 1:  2a4bb60f lib/: Use simple assignment instead of memcpy(3)
2:  1a06250a = 2:  1a06250a lib/string/strlen/: STRNLEN(): Add macro
3:  a85b0961 = 3:  a85b0961 lib/utmp.c: Use countof() for the number of elements of an array
4:  475a76c0 = 4:  475a76c0 lib/utmp.c: Only write ut_syslen if we've written ut_host
5:  813f0912 = 5:  813f0912 lib/: Use STRNLEN() instead of its pattern
6:  5933a030 < -:  -------- lib/typetraits.h: is_array(): Add macro
7:  2b527cb3 ! 6:  fb1c59bd lib/string/strcpy/: MEMCPY(): Add macro
    @@ lib/string/strcpy/memcpy.h (new)
     +
     +#include <string.h>
     +
    -+#include "typetraits.h"
    ++#include "sizeof.h"
     +
     +#include <assert.h>
     +
     +
     +#define MEMCPY(dst, src)                                              \
     +({                                                                    \
    -+  static_assert(is_array(src), "");                             \
    -+  static_assert(is_array(dst), "");                             \
    -+  static_assert(sizeof(dst) == sizeof(src), "");                \
    ++  static_assert(SIZEOF_ARRAY(dst) == SIZEOF_ARRAY(src), "");    \
     +                                                                      \
    -+  memcpy(dst, src, sizeof(dst));                                \
    ++  memcpy(dst, src, SIZEOF_ARRAY(dst));                          \
     +})
     +
     +
8:  166e380a = 7:  0825296f lib/utmp.c: prepare_utmp(): Use MEMCPY() instead of its pattern
v2b
  • Rebase
$ git rd 
1:  2a4bb60f = 1:  21385568 lib/: Use simple assignment instead of memcpy(3)
2:  1a06250a = 2:  4a711c5a lib/string/strlen/: STRNLEN(): Add macro
3:  a85b0961 = 3:  43454a8e lib/utmp.c: Use countof() for the number of elements of an array
4:  475a76c0 = 4:  94f310af lib/utmp.c: Only write ut_syslen if we've written ut_host
5:  813f0912 = 5:  b1698021 lib/: Use STRNLEN() instead of its pattern
6:  fb1c59bd = 6:  1c6b62eb lib/string/strcpy/: MEMCPY(): Add macro
7:  0825296f ! 7:  0fd00afd lib/utmp.c: prepare_utmp(): Use MEMCPY() instead of its pattern
    @@ Commit message
     
      ## lib/utmp.c ##
     @@
    - #include "sizeof.h"
    + #include "string/strchr/strnul.h"
      #include "string/strcmp/streq.h"
      #include "string/strcmp/strprefix.h"
     +#include "string/strcpy/memcpy.h"
v2c
  • Rebase
$ git range-diff 21385568d8a6^..gh/strnlen shadow/master..strnlen 
1:  21385568 = 1:  6571d0fd lib/: Use simple assignment instead of memcpy(3)
2:  4a711c5a = 2:  dc7dcb5b lib/string/strlen/: STRNLEN(): Add macro
3:  43454a8e = 3:  06ada7ec lib/utmp.c: Use countof() for the number of elements of an array
4:  94f310af = 4:  c12b4f12 lib/utmp.c: Only write ut_syslen if we've written ut_host
5:  b1698021 = 5:  622bde1d lib/: Use STRNLEN() instead of its pattern
6:  1c6b62eb = 6:  682b8a6b lib/string/strcpy/: MEMCPY(): Add macro
7:  0fd00afd = 7:  1db02350 lib/utmp.c: prepare_utmp(): Use MEMCPY() instead of its pattern
v3
$ git range-diff shadow/master..gh/strnlen gh/logind_utmp..strnlen 
1:  6571d0fd = 1:  6a1e4f8e lib/: Use simple assignment instead of memcpy(3)
2:  dc7dcb5b = 2:  6ae9cd00 lib/string/strlen/: STRNLEN(): Add macro
3:  06ada7ec ! 3:  770bc281 lib/utmp.c: Use countof() for the number of elements of an array
    @@ Commit message
     
      ## lib/utmp.c ##
     @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host,
    - #endif
    +           STRNCPY(utent->ut_host, hostname);
      #if defined(HAVE_STRUCT_UTMPX_UT_SYSLEN)
                utent->ut_syslen = MIN (strlen (hostname),
     -                                  sizeof (utent->ut_host));
4:  c12b4f12 < -:  -------- lib/utmp.c: Only write ut_syslen if we've written ut_host
5:  622bde1d ! 4:  6404cd44 lib/: Use STRNLEN() instead of its pattern
    @@ lib/string/strdup/xstrndup.h
     
      ## lib/utmp.c ##
     @@
    - #include "string/strcpy/strtcpy.h"
    + #include "string/strdup/strndup.h"
      #include "string/strdup/xstrdup.h"
      #include "string/strdup/xstrndup.h"
    --
    --#ident "$Id$"
     +#include "string/strlen/strnlen.h"
      
      
      #define UTX_LINESIZE  countof(memberof(struct utmpx, ut_line))
     @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host,
    - #if defined(HAVE_STRUCT_UTMPX_UT_HOST)
    +           struct addrinfo *info = NULL;
                STRNCPY(utent->ut_host, hostname);
    - # if defined(HAVE_STRUCT_UTMPX_UT_SYSLEN)
    + #if defined(HAVE_STRUCT_UTMPX_UT_SYSLEN)
     -          utent->ut_syslen = MIN (strlen (hostname),
     -                                  countof(utent->ut_host));
     +          utent->ut_syslen = STRNLEN(utent->ut_host);
    - # endif
      #endif
      #if defined(HAVE_STRUCT_UTMPX_UT_ADDR) || defined(HAVE_STRUCT_UTMPX_UT_ADDR_V6)
    +           if (getaddrinfo (hostname, NULL, NULL, &info) == 0) {
6:  682b8a6b = 5:  213de3d3 lib/string/strcpy/: MEMCPY(): Add macro
7:  1db02350 ! 6:  1218c16f lib/utmp.c: prepare_utmp(): Use MEMCPY() instead of its pattern
    @@ lib/utmp.c
     +#include "string/strcpy/memcpy.h"
      #include "string/strcpy/strncpy.h"
      #include "string/strcpy/strtcpy.h"
    - #include "string/strdup/xstrdup.h"
    + #include "string/strdup/strndup.h"
     @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host,
                        } else if (info->ai_family == AF_INET6) {
                                struct sockaddr_in6 *sa =
v4
  • Move some commits to separate PRs.
$ git range-diff gh/logind_utmp gh/strnlen strnlen 
1:  6a1e4f8e < -:  -------- lib/: Use simple assignment instead of memcpy(3)
2:  6ae9cd00 = 1:  3641970d lib/string/strlen/: STRNLEN(): Add macro
3:  770bc281 = 2:  8dfd9df7 lib/utmp.c: Use countof() for the number of elements of an array
4:  6404cd44 = 3:  d2657a74 lib/: Use STRNLEN() instead of its pattern
5:  213de3d3 < -:  -------- lib/string/strcpy/: MEMCPY(): Add macro
6:  1218c16f < -:  -------- lib/utmp.c: prepare_utmp(): Use MEMCPY() instead of its pattern
v4b
  • Rebase
$ git range-diff gh/logind_utmp..gh/strnlen logind_utmp..strnlen 
1:  35529f23 = 1:  bb829e7f lib/string/strlen/: STRNLEN(): Add macro
2:  bbfdfb4e = 2:  e3dad729 lib/utmp.c: Use countof() for the number of elements of an array
3:  ca7a891f = 3:  a2e44f57 lib/: Use STRNLEN() instead of its pattern
v4c
  • Update lib/string/README.
$ git range-diff gh/logind_utmp gh/strnlen strnlen 
1:  bb829e7f ! 1:  92791d58 lib/string/strlen/: STRNLEN(): Add macro
    @@ lib/Makefile.am: libshadow_la_SOURCES = \
        string/strspn/stpspn.h \
        string/strspn/stprcspn.c \
     
    + ## lib/string/README ##
    +@@ lib/string/README: strsep/ - String separation
    +   Variant of strsep2ls() that allocates the array of strings.
    +   (But the strings themselves are not duplicated.)
    + 
    ++strlen/ - String length
    ++
    ++    STRNLEN()
    ++  Like strnlen(3), but take an array.
    ++
    + strftime.h
    +     STRFTIME()
    +   Like strftime(3), but takes an array.
    +
      ## lib/string/strlen/strnlen.c (new) ##
     @@
     +// SPDX-FileCopyrightText: 2025, Alejandro Colomar <alx@kernel.org>
2:  e3dad729 = 2:  21810b5b lib/utmp.c: Use countof() for the number of elements of an array
3:  a2e44f57 = 3:  f199c2c7 lib/: Use STRNLEN() instead of its pattern
v4d
  • Rebase
$ git range-diff gh/logind_utmp..gh/strnlen logind_utmp..strnlen 
1:  92791d58 = 1:  47ca1fee lib/string/strlen/: STRNLEN(): Add macro
2:  21810b5b = 2:  b0d5de65 lib/utmp.c: Use countof() for the number of elements of an array
3:  f199c2c7 = 3:  979b0aab lib/: Use STRNLEN() instead of its pattern
v4e
  • Rebase
$ git range-diff gh/logind_utmp..gh/strnlen logind_utmp..strnlen 
1:  47ca1fee = 1:  733b33c8 lib/string/strlen/: STRNLEN(): Add macro
2:  b0d5de65 = 2:  8f3dc99d lib/utmp.c: Use countof() for the number of elements of an array
3:  979b0aab = 3:  4c2b5ff7 lib/: Use STRNLEN() instead of its pattern
v4f
  • Rebase
$ git range-diff gh/logind_utmp..gh/strnlen logind_utmp..strnlen 
1:  733b33c8 ! 1:  b1d4fa27 lib/string/strlen/: STRNLEN(): Add macro
    @@ Commit message
     
      ## lib/Makefile.am ##
     @@ lib/Makefile.am: libshadow_la_SOURCES = \
    -   string/strdup/xstrndup.h \
    +   string/strdup/strndup.h \
        string/strftime.c \
        string/strftime.h \
     +  string/strlen/strnlen.c \
2:  8f3dc99d = 2:  950ff602 lib/utmp.c: Use countof() for the number of elements of an array
3:  4c2b5ff7 ! 3:  0dd1f2d2 lib/: Use STRNLEN() instead of its pattern
    @@ Commit message
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    - ## lib/string/strdup/xstrndup.h ##
    -@@
    - #include <string.h>
    - 
    - #include "alloc/x/xmalloc.h"
    --#include "sizeof.h"
    - #include "string/strcpy/strncat.h"
    -+#include "string/strlen/strnlen.h"
    - 
    - 
    - // Similar to strndup(3), but ensure that 's' is an array, and exit on ENOMEM.
    - #define XSTRNDUP(s)                                                           \
    - (                                                                             \
    --  STRNCAT(strcpy(XMALLOC(strnlen(s, countof(s)) + 1, char), ""), s)     \
    -+  STRNCAT(strcpy(XMALLOC(STRNLEN(s) + 1, char), ""), s)                 \
    - )
    - 
    - 
    -
      ## lib/utmp.c ##
     @@
    + #include "string/strcpy/strtcpy.h"
    + #include "string/strdup/strdup.h"
      #include "string/strdup/strndup.h"
    - #include "string/strdup/xstrdup.h"
    - #include "string/strdup/xstrndup.h"
     +#include "string/strlen/strnlen.h"
      
      
v5
  • s/STRNLEN/strnlen_a/
$ git range-diff gh/logind_utmp gh/strnlen strnlen 
1:  b1d4fa27c ! 1:  d66f0f21d lib/string/strlen/: STRNLEN(): Add macro
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/string/strlen/: STRNLEN(): Add macro
    +    lib/string/strlen/: strnlen_a(): Add macro
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    @@ lib/string/README: strsep/ - String separation
      
     +strlen/ - String length
     +
    -+    STRNLEN()
    ++    strnlen_a()
     +  Like strnlen(3), but take an array.
     +
      strftime.h
    @@ lib/string/strlen/strnlen.h (new)
     +#include "sizeof.h"
     +
     +
    -+// nonstring length
    -+#define STRNLEN(strn)  strnlen(strn, countof(strn))
    ++// strnlen_a - nonstring length array
    ++#define strnlen_a(strn)  strnlen(strn, countof(strn))
     +
     +
     +#endif  // include guard
2:  950ff6021 = 2:  6c87538e7 lib/utmp.c: Use countof() for the number of elements of an array
3:  0dd1f2d25 ! 3:  8c6f14aff lib/: Use STRNLEN() instead of its pattern
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/: Use STRNLEN() instead of its pattern
    +    lib/: Use strnlen_a() instead of its pattern
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host,
      #if defined(HAVE_STRUCT_UTMPX_UT_SYSLEN)
     -          utent->ut_syslen = MIN (strlen (hostname),
     -                                  countof(utent->ut_host));
    -+          utent->ut_syslen = STRNLEN(utent->ut_host);
    ++          utent->ut_syslen = strnlen_a(utent->ut_host);
      #endif
      #if defined(HAVE_STRUCT_UTMPX_UT_ADDR) || defined(HAVE_STRUCT_UTMPX_UT_ADDR_V6)
                if (getaddrinfo (hostname, NULL, NULL, &info) == 0) {
v5b
  • Rebase
$ git range-diff gh/logind_utmp..gh/strnlen logind_utmp..strnlen 
1:  d66f0f21d ! 1:  a7e64a3d6 lib/string/strlen/: strnlen_a(): Add macro
    @@ Commit message
     
      ## lib/Makefile.am ##
     @@ lib/Makefile.am: libshadow_la_SOURCES = \
    -   string/strdup/strndup.h \
    +   string/strerrno.h \
        string/strftime.c \
        string/strftime.h \
     +  string/strlen/strnlen.c \
2:  6c87538e7 = 2:  8a31db201 lib/utmp.c: Use countof() for the number of elements of an array
3:  8c6f14aff ! 3:  3613524d8 lib/: Use strnlen_a() instead of its pattern
    @@ Commit message
     
      ## lib/utmp.c ##
     @@
    - #include "string/strcpy/strtcpy.h"
    + #include "string/strcpy/strncpy.h"
      #include "string/strdup/strdup.h"
      #include "string/strdup/strndup.h"
     +#include "string/strlen/strnlen.h"
v5c
  • Rebase
$ git range-diff gh/logind_utmp..gh/strnlen logind_utmp..strnlen --creation-factor=99
1:  a7e64a3d6 ! 1:  563ca9933 lib/string/strlen/: strnlen_a(): Add macro
    @@ lib/string/README: strsep/ - String separation
     +  Like strnlen(3), but take an array.
     +
      strftime.h
    -     STRFTIME()
    +     strftime_a()
        Like strftime(3), but takes an array.
     
      ## lib/string/strlen/strnlen.c (new) ##
2:  8a31db201 ! 2:  476c6cb5f lib/utmp.c: Use countof() for the number of elements of an array
    @@ Commit message
     
      ## lib/utmp.c ##
     @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host,
    -           STRNCPY(utent->ut_host, hostname);
    +           strncpy_a(utent->ut_host, hostname);
      #if defined(HAVE_STRUCT_UTMPX_UT_SYSLEN)
    -           utent->ut_syslen = MIN (strlen (hostname),
    --                                  sizeof (utent->ut_host));
    -+                                  countof(utent->ut_host));
    +           utent->ut_syslen = MIN(strlen(hostname),
    +-                                 sizeof(utent->ut_host));
    ++                                 countof(utent->ut_host));
      #endif
      #if defined(HAVE_STRUCT_UTMPX_UT_ADDR) || defined(HAVE_STRUCT_UTMPX_UT_ADDR_V6)
                if (getaddrinfo (hostname, NULL, NULL, &info) == 0) {
3:  3613524d8 ! 3:  ee77619cb lib/: Use strnlen_a() instead of its pattern
    @@ lib/utmp.c
      #define UTX_LINESIZE  countof(memberof(struct utmpx, ut_line))
     @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host,
                struct addrinfo *info = NULL;
    -           STRNCPY(utent->ut_host, hostname);
    +           strncpy_a(utent->ut_host, hostname);
      #if defined(HAVE_STRUCT_UTMPX_UT_SYSLEN)
    --          utent->ut_syslen = MIN (strlen (hostname),
    --                                  countof(utent->ut_host));
    +-          utent->ut_syslen = MIN(strlen(hostname),
    +-                                 countof(utent->ut_host));
     +          utent->ut_syslen = strnlen_a(utent->ut_host);
      #endif
      #if defined(HAVE_STRUCT_UTMPX_UT_ADDR) || defined(HAVE_STRUCT_UTMPX_UT_ADDR_V6)
v5d
  • Rebase
$ git range-diff gh/logind_utmp..gh/strnlen logind_utmp..strnlen 
1:  563ca9933 = 1:  37a21933a lib/string/strlen/: strnlen_a(): Add macro
2:  476c6cb5f = 2:  21c3dfbde lib/utmp.c: Use countof() for the number of elements of an array
3:  ee77619cb = 3:  11fc0de47 lib/: Use strnlen_a() instead of its pattern
v5e
  • Rebase
$ git range-diff gh/logind_utmp..gh/strnlen logind_utmp..strnlen 
1:  37a21933a = 1:  a057af55e lib/string/strlen/: strnlen_a(): Add macro
2:  21c3dfbde = 2:  8a59b1c94 lib/utmp.c: Use countof() for the number of elements of an array
3:  11fc0de47 = 3:  d61a48cd8 lib/: Use strnlen_a() instead of its pattern
v5f
  • Rebase
$ git range-diff gh/logind_utmp..gh/strnlen logind_utmp..strnlen 
1:  a057af55e = 1:  f2cdd4898 lib/string/strlen/: strnlen_a(): Add macro
2:  8a59b1c94 = 2:  1053b6d93 lib/utmp.c: Use countof() for the number of elements of an array
3:  d61a48cd8 = 3:  2a6a836c3 lib/: Use strnlen_a() instead of its pattern
v5g
  • Rebase
$ git range-diff gh/logind_utmp..gh/strnlen logind_utmp..strnlen 
1:  f2cdd4898 = 1:  f3fdf384f lib/string/strlen/: strnlen_a(): Add macro
2:  1053b6d93 = 2:  629e32ee2 lib/utmp.c: Use countof() for the number of elements of an array
3:  2a6a836c3 = 3:  ff4949a0d lib/: Use strnlen_a() instead of its pattern
v6
  • Rebase
$ git range-diff f3fdf384f^..ff4949a0d master..strnlen 
1:  f3fdf384f = 1:  00786d7f3 lib/string/strlen/: strnlen_a(): Add macro
2:  629e32ee2 ! 2:  a21d2f700 lib/utmp.c: Use countof() for the number of elements of an array
    @@ Commit message
     
      ## lib/utmp.c ##
     @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host,
    -           strncpy_a(utent->ut_host, hostname);
    + #endif
      #if defined(HAVE_STRUCT_UTMPX_UT_SYSLEN)
                utent->ut_syslen = MIN(strlen(hostname),
     -                                 sizeof(utent->ut_host));
3:  ff4949a0d ! 3:  b9160350d lib/: Use strnlen_a() instead of its pattern
    @@ lib/utmp.c
      #include "string/strdup/strndup.h"
     +#include "string/strlen/strnlen.h"
      
    + #ident "$Id$"
      
    - #define UTX_LINESIZE  countof(memberof(struct utmpx, ut_line))
     @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host,
    -           struct addrinfo *info = NULL;
                strncpy_a(utent->ut_host, hostname);
    + #endif
      #if defined(HAVE_STRUCT_UTMPX_UT_SYSLEN)
     -          utent->ut_syslen = MIN(strlen(hostname),
     -                                 countof(utent->ut_host));
v6b
  • Rebase
$ git rd 
1:  00786d7f382e = 1:  ceeb20a051ee lib/string/strlen/: strnlen_a(): Add macro
2:  a21d2f7009b8 = 2:  9a1d1a46576c lib/utmp.c: Use countof() for the number of elements of an array
3:  b9160350d320 ! 3:  d432752934e9 lib/: Use strnlen_a() instead of its pattern
    @@ Commit message
     
      ## lib/utmp.c ##
     @@
    - #include "string/strcpy/strncpy.h"
    + #include "string/strdup/memdup.h"
      #include "string/strdup/strdup.h"
      #include "string/strdup/strndup.h"
     +#include "string/strlen/strnlen.h"
v6c
  • Rebase
$ git rd 
1:  ceeb20a0 = 1:  405153e4 lib/string/strlen/: strnlen_a(): Add macro
2:  9a1d1a46 = 2:  2f936064 lib/utmp.c: Use countof() for the number of elements of an array
3:  d4327529 = 3:  5e699062 lib/: Use strnlen_a() instead of its pattern
v6d
  • Rebase
$ git rd 
1:  405153e4ac2a = 1:  087d8deb40ec lib/string/strlen/: strnlen_a(): Add macro
2:  2f936064eed9 = 2:  8926e78e6ec8 lib/utmp.c: Use countof() for the number of elements of an array
3:  5e6990621e90 ! 3:  4fcee5ebe900 lib/: Use strnlen_a() instead of its pattern
    @@ lib/utmp.c
      #include "string/strdup/strndup.h"
     +#include "string/strlen/strnlen.h"
      
    - #ident "$Id$"
    - 
    + #undef NDEBUG
    + #include <assert.h>
     @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host,
                strncpy_a(utent->ut_host, hostname);
      #endif
v6e
  • Rebase
$ git rd 
1:  087d8deb40ec ! 1:  328f38d8d904 lib/string/strlen/: strnlen_a(): Add macro
    @@ Commit message
     
      ## lib/Makefile.am ##
     @@ lib/Makefile.am: libshadow_la_SOURCES = \
    -   string/strerrno.h \
    +   string/strdup/strndup.h \
        string/strftime.c \
        string/strftime.h \
     +  string/strlen/strnlen.c \
2:  8926e78e6ec8 = 2:  a7a520ce7e8c lib/utmp.c: Use countof() for the number of elements of an array
3:  4fcee5ebe900 = 3:  7c40975d7b8f lib/: Use strnlen_a() instead of its pattern
v6f
  • Rebase
$ git rd 
1:  328f38d8d904 = 1:  46c96c2efd86 lib/string/strlen/: strnlen_a(): Add macro
2:  a7a520ce7e8c = 2:  6a020de438ad lib/utmp.c: Use countof() for the number of elements of an array
3:  7c40975d7b8f = 3:  7d9243b8a5cb lib/: Use strnlen_a() instead of its pattern
v7
  • This actually fixes UB. Document it in the commit message.
$ git rd 
1:  46c96c2efd86 = 1:  46c96c2efd86 lib/string/strlen/: strnlen_a(): Add macro
2:  6a020de438ad = 2:  6a020de438ad lib/utmp.c: Use countof() for the number of elements of an array
3:  7d9243b8a5cb ! 3:  7186a48ad5e6 lib/: Use strnlen_a() instead of its pattern
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/: Use strnlen_a() instead of its pattern
    +    lib/: Use strnlen_a() to fix UB
    +
    +    ut_host need not be a string (it's written with strncpy(3)).
    +    Thus, it's UB to call strlen(3) on it, in case the array has no null
    +    byte.  It probably works by chance, because there might be a null
    +    nearby, but this seems quite clearly a buffer overread.
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
v8
  • It wasn't UB; just not obviously correct.
$ git rd 
1:  46c96c2efd86 = 1:  46c96c2efd86 lib/string/strlen/: strnlen_a(): Add macro
2:  6a020de438ad = 2:  6a020de438ad lib/utmp.c: Use countof() for the number of elements of an array
3:  7186a48ad5e6 ! 3:  788fa0ca3e20 lib/: Use strnlen_a() to fix UB
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/: Use strnlen_a() to fix UB
    +    lib/: Use strnlen_a() instead of its pattern
     
    -    ut_host need not be a string (it's written with strncpy(3)).
    -    Thus, it's UB to call strlen(3) on it, in case the array has no null
    -    byte.  It probably works by chance, because there might be a null
    -    nearby, but this seems quite clearly a buffer overread.
    +    ut_syslen is the length of the nonstring stored in ut_host.  Let's
    +    measure it in the obvious way: strnlen(ut_host, countof(ut_host)), aka,
    +    strnlen_a(ut_host).
    +
    +    MIN(strlen(hostname), countof(ut_host)) works because we've just copied
    +    hostname into ut_host, and the countof() field makes sure we take
    +    truncation into account.  But strnlen_a(ut_host) is just simpler, and
    +    more obviously correct.
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
v9
  • Use <memory.h> instead of <string.h>. Historically, and morally, it's more appropriate. It's quite portable, so it should be fine, even though it's non-standard. See memory.h(3head).
$ git rd 
1:  46c96c2efd86 ! 1:  4431716a4258 lib/string/strlen/: strnlen_a(): Add macro
    @@ lib/string/strlen/strnlen.h (new)
     +
     +#include "config.h"
     +
    -+#include <string.h>
    ++#include <memory.h>
     +
     +#include "sizeof.h"
     +
2:  6a020de438ad = 2:  1631cbf31836 lib/utmp.c: Use countof() for the number of elements of an array
3:  788fa0ca3e20 = 3:  fa7ae640b53f lib/: Use strnlen_a() instead of its pattern
v10
$ git rd --creation-factor=99
1:  4431716a4258 ! 1:  b91863b69c40 lib/string/strlen/: strnlen_a(): Add macro
    @@ Commit message
     
      ## lib/Makefile.am ##
     @@ lib/Makefile.am: libshadow_la_SOURCES = \
    -   string/strdup/strndup.h \
    -   string/strftime.c \
    -   string/strftime.h \
    -+  string/strlen/strnlen.c \
    -+  string/strlen/strnlen.h \
    -   string/strspn/stpspn.c \
    -   string/strspn/stpspn.h \
    -   string/strspn/stprcspn.c \
    +   lockpw.c \
    +   loginprompt.c \
    +   mail.c \
    ++  memory/strnlen/strnlen.c \
    ++  memory/strnlen/strnlen.h \
    +   motd.c \
    +   myname.c \
    +   nss.c \
     
    - ## lib/string/README ##
    -@@ lib/string/README: strsep/ - String separation
    -   Variant of strsep2ls() that allocates the array of strings.
    -   (But the strings themselves are not duplicated.)
    - 
    -+strlen/ - String length
    -+
    -+    strnlen_a()
    -+  Like strnlen(3), but take an array.
    -+
    - strftime.h
    -     strftime_a()
    -   Like strftime(3), but takes an array.
    -
    - ## lib/string/strlen/strnlen.c (new) ##
    + ## lib/memory/strnlen/strnlen.c (new) ##
     @@
    -+// SPDX-FileCopyrightText: 2025, Alejandro Colomar <alx@kernel.org>
    ++// SPDX-FileCopyrightText: 2025-2026, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
     +
     +
     +#include "config.h"
     +
    -+#include "string/strlen/strnlen.h"
    ++#include "memory/strnlen/strnlen.h"
     
    - ## lib/string/strlen/strnlen.h (new) ##
    + ## lib/memory/strnlen/strnlen.h (new) ##
     @@
    -+// SPDX-FileCopyrightText: 2025, Alejandro Colomar <alx@kernel.org>
    ++// SPDX-FileCopyrightText: 2025-2026, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
     +
     +
    -+#ifndef SHADOW_INCLUDE_LIB_STRING_STRLEN_STRNLEN_H_
    -+#define SHADOW_INCLUDE_LIB_STRING_STRLEN_STRNLEN_H_
    ++#ifndef SHADOW_INCLUDE_LIB_MEMORY_STRNLEN_STRNLEN_H_
    ++#define SHADOW_INCLUDE_LIB_MEMORY_STRNLEN_STRNLEN_H_
     +
     +
     +#include "config.h"
    @@ lib/string/strlen/strnlen.h (new)
     +
     +
     +#endif  // include guard
    +
    + ## lib/string/README ##
    +@@ lib/string/README: strsep/ - String separation
    +   Variant of strsep2ls() that allocates the array of strings.
    +   (But the strings themselves are not duplicated.)
    + 
    ++strnlen/ - String length
    ++
    ++    strnlen_a()
    ++  Like strnlen(3), but take an array.
    ++
    + strftime.h
    +     strftime_a()
    +   Like strftime(3), but takes an array.
2:  1631cbf31836 = 2:  87a628d488d5 lib/utmp.c: Use countof() for the number of elements of an array
3:  fa7ae640b53f ! 3:  1d72861c9759 lib/: Use strnlen_a() instead of its pattern
    @@ Commit message
     
      ## lib/utmp.c ##
     @@
    - #include "string/strdup/memdup.h"
    - #include "string/strdup/strdup.h"
    - #include "string/strdup/strndup.h"
    -+#include "string/strlen/strnlen.h"
    - 
    - #undef NDEBUG
    - #include <assert.h>
    + #include "alloc/malloc.h"
    + #include "attr.h"
    + #include "io/syslog.h"
    ++#include "memory/strnlen/strnlen.h"
    + #include "sizeof.h"
    + #include "string/strchr/strnul.h"
    + #include "string/strcmp/streq.h"
     @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host,
                strncpy_a(utent->ut_host, hostname);
      #endif
v10b
  • Rebase
$ git rd 
1:  b91863b69c40 ! 1:  123a3264cd7b lib/string/strlen/: strnlen_a(): Add macro
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/string/strlen/: strnlen_a(): Add macro
    +    lib/memory/strnlen/: strnlen_a(): Add macro
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
2:  87a628d488d5 = 2:  157e20ad4df2 lib/utmp.c: Use countof() for the number of elements of an array
3:  1d72861c9759 = 3:  6e8c56fb6b7c lib/: Use strnlen_a() instead of its pattern

@alejandro-colomar
alejandro-colomar marked this pull request as ready for review July 16, 2025 23:16
@alejandro-colomar
alejandro-colomar force-pushed the strnlen branch 2 times, most recently from b3cf6f6 to fd97871 Compare July 16, 2025 23:34
@alejandro-colomar alejandro-colomar changed the title Simplify code, using STRNLEN() and others Simplify code, using STRNLEN(), MEMCPY(), and others Jul 16, 2025
@alejandro-colomar
alejandro-colomar force-pushed the strnlen branch 2 times, most recently from 83fbc58 to 166e380 Compare July 16, 2025 23:54
@alejandro-colomar
alejandro-colomar force-pushed the strnlen branch 4 times, most recently from 0fd00af to 1db0235 Compare July 18, 2025 20:13
@alejandro-colomar
alejandro-colomar marked this pull request as draft July 22, 2025 10:14
@alejandro-colomar alejandro-colomar changed the title Simplify code, using STRNLEN(), MEMCPY(), and others Simplify code, using STRNLEN(), and others Jul 22, 2025
@alejandro-colomar alejandro-colomar changed the title Simplify code, using STRNLEN(), and others Add and use STRNLEN() instead of more complex code Jul 22, 2025
@alejandro-colomar
alejandro-colomar force-pushed the strnlen branch 2 times, most recently from 9fc9da5 to ca7a891 Compare August 16, 2025 12:37
@alejandro-colomar
alejandro-colomar force-pushed the strnlen branch 2 times, most recently from a2e44f5 to f199c2c Compare September 30, 2025 18:30
@alejandro-colomar
alejandro-colomar force-pushed the strnlen branch 2 times, most recently from 0dd1f2d to 8c6f14a Compare October 28, 2025 09:30
@alejandro-colomar alejandro-colomar changed the title Add and use STRNLEN() instead of more complex code Add and use strnlen_a() instead of more complex code Dec 6, 2025
@alejandro-colomar

Copy link
Copy Markdown
Collaborator Author

Cc: @kees

Comment thread lib/memory/strnlen/strnlen.h
Comment thread lib/string/README
strlen/ - String length

strnlen_a()
Like strnlen(3), but take an array.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you add additional information explaining the benefits of this new API?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The benefits of all _a() macros is that they do the countof() internally, avoiding human mistakes.

@alejandro-colomar alejandro-colomar Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In the case of this one, the mistake is less likely than with other string/nonstring APIs, because the others often have 2 parameters, and one can choose the argument wrongly, while in this case it's more obvious. But it still reduces mistakes a little bit. Since we can take it internally reducing code, it has less mistakes.

@alejandro-colomar alejandro-colomar Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

For example, one thing it does is it makes sure that the input is an array. Nonstrings from utmp are always arrays, so this reduces the chances of accidentally passing a string.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We have some such information at the top of the README:

General guidelines:
===================

-  If there's an upper-case macro that wraps a function, use the macro
   if possible.  These use macro magic to add safety.

-  If there's a *_a() macro that wraps an API, use the macro if
   possible.  These use countof() to add bounds safety.

...

Signed-off-by: Alejandro Colomar <alx@kernel.org>
sizeof() worked because it's an array of char, but it's brittle.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
ut_syslen is the length of the nonstring stored in ut_host.  Let's
measure it in the obvious way: strnlen(ut_host, countof(ut_host)), aka,
strnlen_a(ut_host).

MIN(strlen(hostname), countof(ut_host)) works because we've just copied
hostname into ut_host, and the countof() field makes sure we take
truncation into account.  But strnlen_a(ut_host) is just simpler, and
more obviously correct.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants