search

shadow-maint / shadow

Upstream shadow tree
Other
299 stars 230 forks source link

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

Closed alejandro-colomar closed 3 months ago

alejandro-colomar commented 3 months ago

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 ## 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: Reported-by: Dan Cross @@ 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 @@ 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 -- * SPDX-License-Identifier: BSD-3-Clause -- */ +// SPDX-FileCopyrightText: 2022-2024, Alejandro Colomar +// SPDX-License-Identifier: BSD-3-Clause + - - #include - --#ident "$Id$" -- - #include "string/sprintf.h" - - #include - #include --#include - - - 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 ++ ++#include "string/seprintf.h" ++ ++#include ++ + +#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 -- * SPDX-License-Identifier: BSD-3-Clause -- */ +// SPDX-FileCopyrightText: 2022-2024, Alejandro Colomar +// 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 -@@ 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 ++ ++#include ++#include ++#include ++ ++#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). +// @@ 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 + +#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: Reported-by: Dan Cross + Reviewed-by: Dan Cross Signed-off-by: Alejandro Colomar ## 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 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 @@ 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 - --#include "string/seprintf.h" -+#include "string/sprintf/seprintf.h" - - #include - - - ## lib/string/seprintf.h => lib/string/sprintf/seprintf.h ## - ## lib/string/sprintf/snprintf.c (new) ## @@ +// SPDX-FileCopyrightText: 2023-2024, Alejandro Colomar @@ 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 +- * +- * SPDX-License-Identifier: BSD-3-Clause +- */ ++// SPDX-FileCopyrightText: 2022-2024, Alejandro Colomar ++// SPDX-License-Identifier: BSD-3-Clause + + + #include + +-#if !defined(HAVE_STPEPRINTF) +- +-#ident "$Id$" +- +-#include "string/stpeprintf.h" ++#include "string/sprintf/stpeprintf.h" + + #include + + ++#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 +- * +- * SPDX-License-Identifier: BSD-3-Clause +- */ ++// SPDX-FileCopyrightText: 2022-2024, Alejandro Colomar ++// 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 + +-#if !defined(HAVE_STPEPRINTF) +- + #include + #include + #include + + #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 @@ 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 #include + #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 ## @@ ```
alejandro-colomar commented 3 months ago

Let's close this one for now. The reorganization of dirs is already contained in https://github.com/shadow-maint/shadow/pull/991.