lib/string/sprintf.*, lib/, src/: Rename [v]stpeprintf() to [v]seprintf() #1029

alejandro-colomar · 2024-06-26T20:05:59Z

Dan Cross reported that Plan9 has equivalent functions, called [v]seprint(3). Let's honor Plan9, and reuse their choice of a name, which is more concise (except that we keep the trailing 'f', for consistency with the ISO C printf(3) family of functions).

That might also give some visibility to this API, which is superior to snprintf(3) in some use cases (chaining several calls; catenating formatted strings).

Replace our source code documentation by a reference (and link) to Plan9's [v]seprint(3) manual page.

Link: https://9fans.github.io/plan9port/man/man3/print.html
Reported-by: @dancrossnyc

Revisions:

v2

Keep in a separate file: lib/string/seprintf.[ch]

$ git range-diff shadow/master gh/seprintf seprintf 
1:  0b5d7360 ! 1:  d046d7db lib/string/sprintf.*, lib/, src/: Rename [v]stpeprintf() to >
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/string/sprintf.*, lib/, src/: Rename [v]stpeprintf() to [v]seprintf()
    +    lib/string/seprintf.[ch], lib/, src/: Rename [v]stpeprintf() to [v]seprintf()
     
         Dan Cross reported that Plan9 has equivalent functions, called
         [v]seprint(3).  Let's honor Plan9, and reuse their choice of a name,
    @@ Commit message
         formatted strings).
     
         Replace our source code documentation by a reference (and link) to
    -    Plan9's [v]seprint(3) manual page.  Also, move them to the general
    -    sprintf.[ch] files, together with the rest of the sprintf(3)-like APIs
    -    that we provide.
    +    Plan9's [v]seprint(3) manual page.
     
         Link: <https://9fans.github.io/plan9port/man/man3/print.html>
         Reported-by: Dan Cross <crossd@gmail.com>
    @@ configure.ac: AC_CHECK_FUNCS(arc4random_buf futimes \
     
      ## lib/Makefile.am ##
     @@ lib/Makefile.am: libshadow_la_SOURCES = \
    +   spawn.c \
    +   sssd.c \
    +   sssd.h \
    ++  string/seprintf.c \
    ++  string/seprintf.h \
    +   string/sprintf.c \
        string/sprintf.h \
        string/stpecpy.c \
        string/stpecpy.h \
    @@ lib/idmapping.c
      #include "atoi/str2i.h"
      #include "prototypes.h"
     -#include "string/stpeprintf.h"
    -+#include "string/sprintf.h"
    ++#include "string/seprintf.h"
      #include "idmapping.h"
      #if HAVE_SYS_CAPABILITY_H
      #include <sys/prctl.h>
    @@ lib/idmapping.c: void write_mapping(int proc_dir_fd, int ranges, const struct ma
        }
      
     
    - ## lib/string/sprintf.c ##
    + ## lib/string/seprintf.c (new) ##
     @@
    --/*
    -- * SPDX-FileCopyrightText: 2023, Alejandro Colomar <alx@kernel.org>
    -- * SPDX-License-Identifier: BSD-3-Clause
    -- */
     +// SPDX-FileCopyrightText: 2022-2024, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
     +
    - 
    - #include <config.h>
    - 
    --#ident "$Id$"
    --
    - #include "string/sprintf.h"
    - 
    - #include <stdarg.h>
    - #include <stddef.h>
    --#include <stdio.h>
    - 
    - 
    - extern inline int xasprintf(char **restrict s, const char *restrict fmt, ...);
    -@@ lib/string/sprintf.c: extern inline int snprintf_(char *restrict s, size_t size,
    -     const char *restrict fmt, ...);
    - extern inline int vsnprintf_(char *restrict s, size_t size,
    -     const char *restrict fmt, va_list ap);
    ++
    ++#include <config.h>
    ++
    ++#include "string/seprintf.h"
    ++
    ++#include <stdarg.h>
    ++
     +
     +#if !defined(HAVE_SEPRINTF)
     +extern inline char *seprintf(char *dst, char *end, const char *restrict fmt,
    @@ lib/string/sprintf.c: extern inline int snprintf_(char *restrict s, size_t size,
     +    va_list ap);
     +#endif  // !HAVE_SEPRINTF
     
    - ## lib/string/sprintf.h ##
    + ## lib/string/seprintf.h (new) ##
     @@
    --/*
    -- * SPDX-FileCopyrightText: 2023, Alejandro Colomar <alx@kernel.org>
    -- * SPDX-License-Identifier: BSD-3-Clause
    -- */
     +// SPDX-FileCopyrightText: 2022-2024, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
    - 
    - 
    --#ifndef SHADOW_INCLUDE_LIB_SPRINTF_H_
    --#define SHADOW_INCLUDE_LIB_SPRINTF_H_
    -+#ifndef SHADOW_INCLUDE_LIB_STRING_SPRINTF_H_
    -+#define SHADOW_INCLUDE_LIB_STRING_SPRINTF_H_
    - 
    - 
    - #include <config.h>
    -@@ lib/string/sprintf.h: format_attr(printf, 3, 0)
    - inline int vsnprintf_(char *restrict s, size_t size, const char *restrict fmt,
    -     va_list ap);
    - 
    ++
    ++
    ++#ifndef SHADOW_INCLUDE_LIB_STRING_SPRINTF_SEPRINTF_H_
    ++#define SHADOW_INCLUDE_LIB_STRING_SPRINTF_SEPRINTF_H_
    ++
    ++
    ++#include <config.h>
    ++
    ++#include <stdarg.h>
    ++#include <stddef.h>
    ++#include <stdio.h>
    ++
    ++#include "attr.h"
    ++#include "defines.h"
    ++
    ++
     +#if !defined(HAVE_SEPRINTF)
     +format_attr(printf, 3, 4)
     +inline char *seprintf(char *dst, char *end, const char *restrict fmt, ...);
    @@ lib/string/sprintf.h: format_attr(printf, 3, 0)
     +    va_list ap);
     +#endif  // !HAVE_SEPRINTF
     +
    - 
    - inline int
    - xasprintf(char **restrict s, const char *restrict fmt, ...)
    -@@ lib/string/sprintf.h: vsnprintf_(char *restrict s, size_t size, const char *rest>
    - }
    - 
    - 
    ++
     +#if !defined(HAVE_SEPRINTF)
     +// Equivalent to Plan9's seprint(3).
     +// <https://9fans.github.io/plan9port/man/man3/print.html>
    @@ lib/string/sprintf.h: vsnprintf_(char *restrict s, size_t size, const char *rest
     +#endif  // !HAVE_SEPRINTF
     +
     +
    - #endif  // include guard
    ++#endif  // include guard
     
      ## lib/string/stpecpy.h ##
     @@ lib/string/stpecpy.h: inline char *stpecpy(char *dst, char *end, const char *res>
    @@ lib/string/stpeprintf.h (deleted)
     
      ## src/groupmod.c ##
     @@
    + #include "sgroupio.h"
      #endif
      #include "shadowlog.h"
    ++#include "string/seprintf.h"
      #include "string/stpecpy.h"
     -#include "string/stpeprintf.h"
    -+#include "string/sprintf.h"
      /*
       * exit status values
       */

v3

Remove unnecessary include
Re-organize lib/string/ into subdirectories

$ git range-diff master gh/seprintf seprintf 
1:  d046d7db ! 1:  4cfbb930 lib/string/seprintf.[ch], lib/, src/: Rename [v]stpeprintf() to [v]seprintf()
    @@ lib/string/seprintf.h (new)
     +#include <stdio.h>
     +
     +#include "attr.h"
    -+#include "defines.h"
     +
     +
     +#if !defined(HAVE_SEPRINTF)
-:  -------- > 2:  2632bc76 lib/string/sprintf/, lib/, src/, tests/: Move all sprintf(3)-like APIs to a subdirectory
-:  -------- > 3:  0b7e81c0 lib/string/strcpy/, lib/, src/, tests/: Move all copying APIs to a subdirectory

v3b

@dancrossnyc reviewed the first patch

$ git range-diff master gh/seprintf seprintf 
1:  4cfbb930 ! 1:  abdf883b lib/string/seprintf.[ch], lib/, src/: Rename [v]stpeprintf() to [v]seprintf()
    @@ Commit message
     
         Link: <https://9fans.github.io/plan9port/man/man3/print.html>
         Reported-by: Dan Cross <crossd@gmail.com>
    +    Reviewed-by: Dan Cross <crossd@gmail.com>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## configure.ac ##
2:  2632bc76 = 2:  4db1536e lib/string/sprintf/, lib/, src/, tests/: Move all sprintf(3)-like APIs to a subdirectory
3:  0b7e81c0 = 3:  09873b92 lib/string/strcpy/, lib/, src/, tests/: Move all copying APIs to a subdirectory

v3c

Fix file name

$ git range-diff master gh/seprintf seprintf 
1:  abdf883b = 1:  abdf883b lib/string/seprintf.[ch], lib/, src/: Rename [v]stpeprintf() to [v]seprintf()
2:  4db1536e ! 2:  66b96d18 lib/string/sprintf/, lib/, src/, tests/: Move all sprintf(3)-like APIs to a subdirectory
    @@ tests/unit/Makefile.am: test_strtcpy_LDADD = \
          $(NULL)
      test_xasprintf_CFLAGS = \
     
    - ## tests/unit/test_sprintf.c ##
    + ## tests/unit/test_sprintf.c => tests/unit/test_snprintf.c ##
     @@
      #include <cmocka.h>
      
3:  09873b92 = 3:  90565c02 lib/string/strcpy/, lib/, src/, tests/: Move all copying APIs to a subdirectory

v3d

Rebase

$ git range-diff gh/master..gh/seprintf master..seprintf 
1:  abdf883b = 1:  c13092b6 lib/string/seprintf.[ch], lib/, src/: Rename [v]stpeprintf() to [v]seprintf()
2:  66b96d18 = 2:  dc6da914 lib/string/sprintf/, lib/, src/, tests/: Move all sprintf(3)-like APIs to a subdirectory
3:  90565c02 = 3:  0bbfc60f lib/string/strcpy/, lib/, src/, tests/: Move all copying APIs to a subdirectory

v4

Do not rename the API. Just move the files around for a better organization. The Plan9 API cannot detect truncation. If Plan9 improves their API, we'll rename ours in the future. (Cc: @hallyn )

$ git range-diff gh/master gh/seprintf seprintf 
1:  c13092b6 < -:  -------- lib/string/seprintf.[ch], lib/, src/: Rename [v]stpeprintf() to [v]seprintf()
2:  dc6da914 ! 1:  a77a6a59 lib/string/sprintf/, lib/, src/, tests/: Move all sprintf(3)-like APIs to a subdirectory
    @@ lib/Makefile.am: libshadow_la_SOURCES = \
        spawn.c \
        sssd.c \
        sssd.h \
    --  string/seprintf.c \
    --  string/seprintf.h \
     -  string/sprintf.c \
     -  string/sprintf.h \
    -+  string/sprintf/seprintf.c \
    -+  string/sprintf/seprintf.h \
     +  string/sprintf/snprintf.c \
     +  string/sprintf/snprintf.h \
    ++  string/sprintf/stpeprintf.c \
    ++  string/sprintf/stpeprintf.h \
     +  string/sprintf/xasprintf.c \
     +  string/sprintf/xasprintf.h \
        string/stpecpy.c \
        string/stpecpy.h \
    +-  string/stpeprintf.c \
    +-  string/stpeprintf.h \
        string/strftime.c \
    +   string/strftime.h \
    +   string/strncpy.h \
     
      ## lib/commonio.c ##
     @@
    @@ lib/idmapping.c
      #include "alloc.h"
      #include "atoi/str2i.h"
      #include "prototypes.h"
    --#include "string/seprintf.h"
    -+#include "string/sprintf/seprintf.h"
    +-#include "string/stpeprintf.h"
    ++#include "string/sprintf/stpeprintf.h"
      #include "idmapping.h"
      #if HAVE_SYS_CAPABILITY_H
      #include <sys/prctl.h>
    @@ lib/string/sprintf.c (deleted)
     -extern inline int vsnprintf_(char *restrict s, size_t size,
     -    const char *restrict fmt, va_list ap);
     
    - ## lib/string/seprintf.c => lib/string/sprintf/seprintf.c ##
    -@@
    - 
    - #include <config.h>
    - 
    --#include "string/seprintf.h"
    -+#include "string/sprintf/seprintf.h"
    - 
    - #include <stdarg.h>
    - 
    -
    - ## lib/string/seprintf.h => lib/string/sprintf/seprintf.h ##
    -
      ## lib/string/sprintf/snprintf.c (new) ##
     @@
     +// SPDX-FileCopyrightText: 2023-2024, Alejandro Colomar <alx@kernel.org>
    @@ lib/string/sprintf/snprintf.h: inline int vsnprintf_(char *restrict s, size_t si
      snprintf_(char *restrict s, size_t size, const char *restrict fmt, ...)
      {
     
    + ## lib/string/stpeprintf.c => lib/string/sprintf/stpeprintf.c ##
    +@@
    +-/*
    +- * SPDX-FileCopyrightText:  2022 - 2023, Alejandro Colomar <alx@kernel.org>
    +- *
    +- * SPDX-License-Identifier:  BSD-3-Clause
    +- */
    ++// SPDX-FileCopyrightText: 2022-2024, Alejandro Colomar <alx@kernel.org>
    ++// SPDX-License-Identifier: BSD-3-Clause
    + 
    + 
    + #include <config.h>
    + 
    +-#if !defined(HAVE_STPEPRINTF)
    +-
    +-#ident "$Id$"
    +-
    +-#include "string/stpeprintf.h"
    ++#include "string/sprintf/stpeprintf.h"
    + 
    + #include <stdarg.h>
    + 
    + 
    ++#if !defined(HAVE_STPEPRINTF)
    + extern inline char *stpeprintf(char *dst, char *end, const char *restrict fmt,
    +     ...);
    + extern inline char *vstpeprintf(char *dst, char *end, const char *restrict fmt,
    +     va_list ap);
    +-
    +-
    +-#endif  // !HAVE_STPEPRINTF
    ++#endif
    +
    + ## lib/string/stpeprintf.h => lib/string/sprintf/stpeprintf.h ##
    +@@
    +-/*
    +- * SPDX-FileCopyrightText:  2022 - 2023, Alejandro Colomar <alx@kernel.org>
    +- *
    +- * SPDX-License-Identifier:  BSD-3-Clause
    +- */
    ++// SPDX-FileCopyrightText: 2022-2024, Alejandro Colomar <alx@kernel.org>
    ++// SPDX-License-Identifier: BSD-3-Clause
    + 
    + 
    +-#ifndef SHADOW_INCLUDE_LIB_STPEPRINTF_H_
    +-#define SHADOW_INCLUDE_LIB_STPEPRINTF_H_
    ++#ifndef SHADOW_INCLUDE_LIB_STRING_SPRINTF_STPEPRINTF_H_
    ++#define SHADOW_INCLUDE_LIB_STRING_SPRINTF_STPEPRINTF_H_
    + 
    + 
    + #include <config.h>
    + 
    +-#if !defined(HAVE_STPEPRINTF)
    +-
    + #include <stdarg.h>
    + #include <stddef.h>
    + #include <stdio.h>
    + 
    + #include "attr.h"
    +-#include "defines.h"
    + 
    + 
    ++#if !defined(HAVE_STPEPRINTF)
    + format_attr(printf, 3, 4)
    + inline char *stpeprintf(char *dst, char *end, const char *restrict fmt, ...);
    +-
    + format_attr(printf, 3, 0)
    + inline char *vstpeprintf(char *dst, char *end, const char *restrict fmt,
    +     va_list ap);
    ++#endif
    + 
    + 
    + /*
    +@@ lib/string/sprintf/stpeprintf.h: inline char *vstpeprintf(char *dst, char *end, const char *restrict fmt,
    +  */
    + 
    + 
    ++#if !defined(HAVE_STPEPRINTF)
    + inline char *
    + stpeprintf(char *dst, char *end, const char *restrict fmt, ...)
    + {
    +@@ lib/string/sprintf/stpeprintf.h: stpeprintf(char *dst, char *end, const char *restrict fmt, ...)
    + 
    +   return p;
    + }
    ++#endif
    + 
    + 
    ++#if !defined(HAVE_STPEPRINTF)
    + inline char *
    + vstpeprintf(char *dst, char *end, const char *restrict fmt, va_list ap)
    + {
    +@@ lib/string/sprintf/stpeprintf.h: vstpeprintf(char *dst, char *end, const char *restrict fmt, va_list ap)
    + 
    +   return dst + len;
    + }
    ++#endif
    + 
    + 
    +-#endif  // !HAVE_STPEPRINTF
    + #endif  // include guard
    +
      ## lib/string/sprintf/xasprintf.c (new) ##
     @@
     +// SPDX-FileCopyrightText: 2023-2024, Alejandro Colomar <alx@kernel.org>
    @@ src/groupmod.c
      #include "sgroupio.h"
      #endif
      #include "shadowlog.h"
    --#include "string/seprintf.h"
    -+#include "string/sprintf/seprintf.h"
    ++#include "string/sprintf/stpeprintf.h"
      #include "string/stpecpy.h"
    +-#include "string/stpeprintf.h"
      /*
       * exit status values
    +  */
     
      ## src/login.c ##
     @@
3:  0bbfc60f ! 2:  8968a7b5 lib/string/strcpy/, lib/, src/, tests/: Move all copying APIs to a subdirectory
    @@ Commit message
     
      ## lib/Makefile.am ##
     @@ lib/Makefile.am: libshadow_la_SOURCES = \
    -   string/sprintf/snprintf.h \
    +   string/sprintf/stpeprintf.h \
        string/sprintf/xasprintf.c \
        string/sprintf/xasprintf.h \
     -  string/stpecpy.c \
    @@ src/groupmod.c
     @@
      #endif
      #include "shadowlog.h"
    - #include "string/sprintf/seprintf.h"
    + #include "string/sprintf/stpeprintf.h"
     -#include "string/stpecpy.h"
     +#include "string/strcpy/stpecpy.h"
     +

v4b

Rebase

$ git range-diff gh/master..gh/seprintf shadow/master..seprintf 
1:  a77a6a59 ! 1:  9c7d1b78 lib/string/sprintf/, lib/, src/, tests/: Move all sprintf(3)-like APIs to a subdirectory
    @@ lib/env.c
     
      ## lib/get_pid.c ##
     @@
    - #include <sys/stat.h>
      #include <fcntl.h>
      
    + #include "atoi/getnum.h"
     -#include "string/sprintf.h"
     +#include "string/sprintf/snprintf.h"
      
      
    - int
    + /*
     
      ## lib/getdef.c ##
     @@
    @@ src/groupmod.c
     +#include "string/sprintf/stpeprintf.h"
      #include "string/stpecpy.h"
     -#include "string/stpeprintf.h"
    + 
    + 
      /*
    -  * exit status values
    -  */
     
      ## src/login.c ##
     @@
    @@ tests/unit/Makefile.am: check_PROGRAMS = \
     +    test_snprintf \
          test_strncpy \
          test_strtcpy \
    -     test_xasprintf \
    +     test_typetraits \
     @@ tests/unit/Makefile.am: test_logind_LDADD = \
          $(LIBSYSTEMD) \
          $(NULL)
    @@ tests/unit/Makefile.am: test_logind_LDADD = \
          $(CMOCKA_LIBS) \
          $(NULL)
      
    -@@ tests/unit/Makefile.am: test_strtcpy_LDADD = \
    +@@ tests/unit/Makefile.am: test_typetraits_LDADD = \
          $(NULL)
      
      test_xasprintf_SOURCES = \
2:  8968a7b5 ! 2:  66eeb817 lib/string/strcpy/, lib/, src/, tests/: Move all copying APIs to a subdirectory
    @@ src/groupmod.c
      #include "string/sprintf/stpeprintf.h"
     -#include "string/stpecpy.h"
     +#include "string/strcpy/stpecpy.h"
    -+
    -+
    + 
    + 
      /*
    -  * exit status values
    -  */
     
      ## src/login.c ##
     @@

dancrossnyc

Looks good to me! Thanks for the shout-out. :-)

…s to a subdirectory And have a separate file for each pair of APIs. Signed-off-by: Alejandro Colomar <alx@kernel.org>

…bdirectory Signed-off-by: Alejandro Colomar <alx@kernel.org>

alejandro-colomar · 2024-06-30T22:57:27Z

Let's close this one for now. The reorganization of dirs is already contained in #991.

alejandro-colomar force-pushed the seprintf branch from 85c566f to 096e728 Compare June 26, 2024 20:08

alejandro-colomar marked this pull request as ready for review June 26, 2024 20:10

alejandro-colomar force-pushed the seprintf branch 2 times, most recently from 6e69699 to 0b5d736 Compare June 26, 2024 21:07

dancrossnyc approved these changes Jun 26, 2024

View reviewed changes

alejandro-colomar requested review from hallyn and ikerexxe June 26, 2024 22:56

alejandro-colomar force-pushed the seprintf branch 2 times, most recently from d046d7d to 0b7e81c Compare June 27, 2024 09:51

alejandro-colomar mentioned this pull request Jun 27, 2024

Simplify a lot of NIH stuff, and fix several bugs #991

Merged

alejandro-colomar force-pushed the seprintf branch 3 times, most recently from 90565c0 to 0bbfc60 Compare June 29, 2024 16:48

alejandro-colomar marked this pull request as draft June 29, 2024 18:11

alejandro-colomar force-pushed the seprintf branch from 0bbfc60 to 8968a7b Compare June 30, 2024 22:41

alejandro-colomar marked this pull request as ready for review June 30, 2024 22:43

alejandro-colomar added 2 commits July 1, 2024 00:50

lib/string/sprintf/, lib/, src/, tests/: Move all sprintf(3)-like API…

9c7d1b7

…s to a subdirectory And have a separate file for each pair of APIs. Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/string/strcpy/, lib/, src/, tests/: Move all copying APIs to a su…

66eeb81

…bdirectory Signed-off-by: Alejandro Colomar <alx@kernel.org>

alejandro-colomar closed this Jun 30, 2024

alejandro-colomar reopened this Jun 30, 2024

alejandro-colomar force-pushed the seprintf branch from 8968a7b to 66eeb81 Compare June 30, 2024 22:58

alejandro-colomar closed this Jun 30, 2024

alejandro-colomar deleted the seprintf branch July 2, 2024 10:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib/string/sprintf.*, lib/, src/: Rename [v]stpeprintf() to [v]seprintf() #1029

lib/string/sprintf.*, lib/, src/: Rename [v]stpeprintf() to [v]seprintf() #1029

alejandro-colomar commented Jun 26, 2024 •

edited

Loading

dancrossnyc left a comment

alejandro-colomar commented Jun 30, 2024

lib/string/sprintf.*, lib/, src/: Rename [v]stpeprintf() to [v]seprintf() #1029

lib/string/sprintf.*, lib/, src/: Rename [v]stpeprintf() to [v]seprintf() #1029

Conversation

alejandro-colomar commented Jun 26, 2024 • edited Loading

dancrossnyc left a comment

Choose a reason for hiding this comment

alejandro-colomar commented Jun 30, 2024

alejandro-colomar commented Jun 26, 2024 •

edited

Loading