lib/: Add ispfchar(), and use it instead of its pattern by alejandro-colomar · Pull Request #1469 · shadow-maint/shadow

alejandro-colomar · 2026-01-01T00:40:07Z

Queued after Add C-locale variants of isascii(3) APIs, and use them #1471.

Revisions:

v2

Move ispfchar() under lib/string/.
Document it in lib/string/README.

$ git rd 
1:  247466ab6 ! 1:  85d3d8cb4 lib/ctype/: ispfchar(): Add function
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/ctype/: ispfchar(): Add function
    +    lib/string/ctype/isascii/: ispfchar(): Add function
     
         This function returns true if the input character is a character from
         the POSIX portable filename character set.
    @@ Commit message
     
      ## lib/Makefile.am ##
     @@ lib/Makefile.am: libshadow_la_SOURCES = \
    -   console.c \
    -   copydir.c \
    -   csrand.c \
    -+  ctype/ispfchar.c \
    -+  ctype/ispfchar.h \
    -   defines.h \
    -   encrypt.c \
    -   env.c \
    +   spawn.c \
    +   sssd.c \
    +   sssd.h \
    ++  string/ctype/isascii/ispfchar.c \
    ++  string/ctype/isascii/ispfchar.h \
    +   string/ctype/strchrisascii/strchriscntrl.c \
    +   string/ctype/strchrisascii/strchriscntrl.h \
    +   string/ctype/strisascii/strisdigit.c \
     
    - ## lib/ctype/ispfchar.c (new) ##
    + ## lib/string/README ##
    +@@ lib/string/README: Specific guidelines:
    + 
    + ctype/ - Character classification and conversion functions
    + 
    ++    isascii/
    ++  The functions defined under this directory
    ++  return true
    ++  is the character
    ++  belongs to the category specified in the function name.
    ++
    +     strchrisascii/
    +   The functions defined under this directory
    +   return true
    +
    + ## lib/string/ctype/isascii/ispfchar.c (new) ##
     @@
    -+// SPDX-FileCopyrightText: 2024, Alejandro Colomar <alx@kernel.org>
    ++// SPDX-FileCopyrightText: 2024-2025, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
     +
     +
     +#include <config.h>
     +
    -+#include "ctype/ispfchar.h"
    ++#include "string/ctype/isascii/ispfchar.h"
     +
     +#include <stdbool.h>
     +
     +
     +extern inline bool ispfchar(unsigned char c);
     
    - ## lib/ctype/ispfchar.h (new) ##
    + ## lib/string/ctype/isascii/ispfchar.h (new) ##
     @@
    -+// SPDX-FileCopyrightText: 2024, Alejandro Colomar <alx@kernel.org>
    ++// SPDX-FileCopyrightText: 2024-2025, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
     +
     +
    -+#ifndef SHADOW_INCLUDE_LIB_CTYPE_ISPFCHAR_H_
    -+#define SHADOW_INCLUDE_LIB_CTYPE_ISPFCHAR_H_
    ++#ifndef SHADOW_INCLUDE_LIB_STRING_CTYPE_ISASCII_ISPFCHAR_H_
    ++#define SHADOW_INCLUDE_LIB_STRING_CTYPE_ISASCII_ISPFCHAR_H_
     +
     +
     +#include <config.h>
2:  219955397 = 2:  1b917a73d lib/chkname.c: is_valid_name(): Use isalnum(3) instead of its pattern
3:  92332a203 = 3:  e2d77042a lib/chkname.c: is_valid_name(): Split Samba check
4:  1ead5c62f ! 4:  8fd9e48f6 lib/chkname.c: is_valid_name(): Use ispfchar() to simplify
    @@ lib/chkname.c
      
      #include "defines.h"
      #include "chkname.h"
    -+#include "ctype/ispfchar.h"
    ++#include "string/ctype/isascii/ispfchar.h"
      #include "string/ctype/strchrisascii/strchriscntrl.h"
      #include "string/ctype/strisascii/strisdigit.h"
      #include "string/strcmp/streq.h"

v2b

tfix

$ git rd 
1:  85d3d8cb4 ! 1:  d339ea77e lib/string/ctype/isascii/: ispfchar(): Add function
    @@ lib/string/README: Specific guidelines:
     +    isascii/
     +  The functions defined under this directory
     +  return true
    -+  is the character
    ++  if the character
     +  belongs to the category specified in the function name.
     +
          strchrisascii/
2:  1b917a73d = 2:  7f83c18fe lib/chkname.c: is_valid_name(): Use isalnum(3) instead of its pattern
3:  e2d77042a = 3:  4c7cee7bc lib/chkname.c: is_valid_name(): Split Samba check
4:  8fd9e48f6 = 4:  d39008cae lib/chkname.c: is_valid_name(): Use ispfchar() to simplify

v3

Rebase after Add C-locale variants of isascii(3) APIs, and use them #1471.

$ git range-diff master..gh/chkname gh/isascii_c..chkname 
1:  d339ea77e < -:  --------- lib/string/ctype/isascii/: ispfchar(): Add function
-:  --------- > 1:  02df1aa64 lib/string/ctype/isascii.h: ispfchar_c(): Add function
2:  7f83c18fe = 2:  4259b5bba lib/chkname.c: is_valid_name(): Use isalnum(3) instead of its pattern
3:  4c7cee7bc = 3:  69076fb30 lib/chkname.c: is_valid_name(): Split Samba check
4:  d39008cae ! 4:  1cabaf3f8 lib/chkname.c: is_valid_name(): Use ispfchar() to simplify
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/chkname.c: is_valid_name(): Use ispfchar() to simplify
    +    lib/chkname.c: is_valid_name(): Use ispfchar_c() to simplify
     
         In the first case, we can do the transformation because a few lines
         above, we explicitly reject a name starting with a '-'.
     
    -    In the second case, we're obviously using ispfchar() instead of its
    +    In the second case, we're obviously using ispfchar_c() instead of its
         pattern.
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
    @@ lib/chkname.c
      
      #include "defines.h"
      #include "chkname.h"
    -+#include "string/ctype/isascii/ispfchar.h"
    - #include "string/ctype/strchrisascii/strchriscntrl.h"
    - #include "string/ctype/strisascii/strisdigit.h"
    ++#include "string/ctype/isascii.h"
    + #include "string/ctype/strchrisascii.h"
    + #include "string/ctype/strisascii.h"
      #include "string/strcmp/streq.h"
     @@ lib/chkname.c: is_valid_name(const char *name)
         * sake of Samba 3.x "add machine script"
    @@ lib/chkname.c: is_valid_name(const char *name)
     -        *name == '_' ||
     -        *name == '.'))
     -  {
    -+  if (!ispfchar(*name)) {
    ++  if (!ispfchar_c(*name)) {
                errno = EILSEQ;
                return false;
        }
    @@ lib/chkname.c: is_valid_name(const char *name)
     -                *name == '-'
     -               ))
     -          {
    -+          if (!ispfchar(*name)) {
    ++          if (!ispfchar_c(*name)) {
                        errno = EILSEQ;
                        return false;
                }

v3b

Rebase

$ git range-diff 02df1aa649b7^..gh/chkname isascii_c..chkname 
1:  02df1aa64 = 1:  04d41a43c lib/string/ctype/isascii.h: ispfchar_c(): Add function
2:  4259b5bba = 2:  f15392da0 lib/chkname.c: is_valid_name(): Use isalnum(3) instead of its pattern
3:  69076fb30 = 3:  ecccb9861 lib/chkname.c: is_valid_name(): Split Samba check
4:  1cabaf3f8 = 4:  e371b465b lib/chkname.c: is_valid_name(): Use ispfchar_c() to simplify

v3c

Fix typo.

$ git range-diff gh/isascii_c gh/chkname chkname 
1:  04d41a43c ! 1:  d9c21d8e5 lib/string/ctype/isascii.h: ispfchar_c(): Add function
    @@ lib/string/ctype/isascii.h
      #define CTYPE_PRINT_C   CTYPE_GRAPH_C " "
      #define CTYPE_XDIGIT_C  CTYPE_DIGIT_C "abcdefABCDEF"
      #define CTYPE_ASCII_C   CTYPE_PRINT_C CTYPE_CNTRL_C /*NUL*/
    -+#define CTYPE_PFCHAR_C  CTYPE_ALNUM "._-"  // portable filename character set
    ++#define CTYPE_PFCHAR_C  CTYPE_ALNUM_C "._-"  // portable filename character set
      
      
      // isascii_c - is [:ascii:] C-locale
2:  f15392da0 = 2:  44af79d71 lib/chkname.c: is_valid_name(): Use isalnum(3) instead of its pattern
3:  ecccb9861 = 3:  ea9364860 lib/chkname.c: is_valid_name(): Split Samba check
4:  e371b465b = 4:  000c8c170 lib/chkname.c: is_valid_name(): Use ispfchar_c() to simplify

v3d

Rebase

$ git range-diff gh/isascii_c..gh/chkname isascii_c..chkname 
1:  d9c21d8e5e4c = 1:  31e007269215 lib/string/ctype/isascii.h: ispfchar_c(): Add function
2:  44af79d71b98 = 2:  f22a677577ec lib/chkname.c: is_valid_name(): Use isalnum(3) instead of its pattern
3:  ea9364860219 = 3:  5038f83258cb lib/chkname.c: is_valid_name(): Split Samba check
4:  000c8c170d17 = 4:  ed15a3cc0786 lib/chkname.c: is_valid_name(): Use ispfchar_c() to simplify

v3e

Rebase

$ git range-diff gh/isascii_c..gh/chkname isascii_c..chkname 
1:  31e00726 = 1:  52ecb2ee lib/string/ctype/isascii.h: ispfchar_c(): Add function
2:  f22a6775 = 2:  f8883e1b lib/chkname.c: is_valid_name(): Use isalnum(3) instead of its pattern
3:  5038f832 = 3:  8c2b031d lib/chkname.c: is_valid_name(): Split Samba check
4:  ed15a3cc = 4:  42b2e85b lib/chkname.c: is_valid_name(): Use ispfchar_c() to simplify

v3f

Rebase

$ git range-diff gh/isascii_c..gh/chkname isascii_c..chkname 
1:  52ecb2ee = 1:  e29f3481 lib/string/ctype/isascii.h: ispfchar_c(): Add function
2:  f8883e1b = 2:  4c8ed46a lib/chkname.c: is_valid_name(): Use isalnum(3) instead of its pattern
3:  8c2b031d = 3:  831be018 lib/chkname.c: is_valid_name(): Split Samba check
4:  42b2e85b = 4:  d3f1d69e lib/chkname.c: is_valid_name(): Use ispfchar_c() to simplify

v4

Rebase

$ git range-diff --creation-factor=99 gh/isascii_c..gh/chkname isascii_c..chkname 
1:  e29f3481 ! 1:  a78a389c lib/string/ctype/isascii.h: ispfchar_c(): Add function
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/string/ctype/isascii.h: ispfchar_c(): Add function
    +    lib/string/ctype/isascii.h: ispfchar_c(): Add macro
     
         This function returns true if the input character is a character from
         the POSIX portable filename character set.
    @@ Commit message
     
      ## lib/string/ctype/isascii.h ##
     @@
    - #define CTYPE_PRINT_C   CTYPE_GRAPH_C " "
    - #define CTYPE_XDIGIT_C  CTYPE_DIGIT_C "abcdefABCDEF"
    - #define CTYPE_ASCII_C   CTYPE_PRINT_C CTYPE_CNTRL_C /*NUL*/
    -+#define CTYPE_PFCHAR_C  CTYPE_ALNUM_C "._-"  // portable filename character set
    + #define CTYPE_PRINT_C     CTYPE_GRAPH_C " "
    + #define CTYPE_XDIGIT_C    CTYPE_DIGIT_C "abcdefABCDEF"
    + #define CTYPE_ASCII_C     CTYPE_PRINT_C CTYPE_CNTRL_C /*NUL*/
    ++#define CTYPE_PFCHAR_C    CTYPE_ALNUM_C "._-"  // portable filename character set
      
    - 
    - // isascii_c - is [:ascii:] C-locale
    + #define CTYPE_CNTRL_C0    CTYPE_CNTRL_C
    + #define CTYPE_CNTRL_C1                                                \
     @@
    - #define isgraph_c(c)   (!streq(strchrnul(CTYPE_GRAPH_C, c), ""))
    - #define isprint_c(c)   (!streq(strchrnul(CTYPE_PRINT_C, c), ""))
    - #define isxdigit_c(c)  (!streq(strchrnul(CTYPE_XDIGIT_C, c), ""))
    -+#define ispfchar_c(c)  (!streq(strchrnul(CTYPE_PFCHAR_C, c), ""))
    + #define isgraph_c(c)      (!streq(strchrnul(CTYPE_GRAPH_C, c), ""))
    + #define isprint_c(c)      (!streq(strchrnul(CTYPE_PRINT_C, c), ""))
    + #define isxdigit_c(c)     (!streq(strchrnul(CTYPE_XDIGIT_C, c), ""))
    ++#define ispfchar_c(c)     (!streq(strchrnul(CTYPE_PFCHAR_C, c), ""))
      
    - 
    - #endif  // include guard
    + // iscntrl_c0c1 - is control-character (C0 or C1)
    + #define iscntrl_c0c1(c)   (!!strchr(CTYPE_CNTRL_C0C1, c))
2:  4c8ed46a = 2:  08d72ce0 lib/chkname.c: is_valid_name(): Use isalnum(3) instead of its pattern
3:  831be018 = 3:  82202d26 lib/chkname.c: is_valid_name(): Split Samba check
4:  d3f1d69e ! 4:  4f120ee7 lib/chkname.c: is_valid_name(): Use ispfchar_c() to simplify
    @@ Commit message
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## lib/chkname.c ##
    -@@
    - 
    - #include "defines.h"
    - #include "chkname.h"
    -+#include "string/ctype/isascii.h"
    - #include "string/ctype/strchrisascii.h"
    - #include "string/ctype/strisascii.h"
    - #include "string/strcmp/streq.h"
     @@ lib/chkname.c: is_valid_name(const char *name)
         * sake of Samba 3.x "add machine script"
         */

lib/string/ctype/isascii/ispfchar.h

lib/chkname.c

Resolved.

These are like the isascii(3) family of APIs, but use the C locale, as the _c suffix hints. These macros behave well with non-casted input, unlike isascii(3). The isascii_c() and iscntrl_c() implementations are different from the rest because they must return true for '\0'. Reported-by: Tobias Stoeckmann <tobias@stoeckmann.org> Cc: Paul Eggert <eggert@cs.ucla.edu> Signed-off-by: Alejandro Colomar <alx@kernel.org>

This matches the two categories (C0, C1) of control characters. Link: <https://en.wikipedia.org/wiki/C0_and_C1_control_codes> Cc: KhaelK-Praetorian <khael.kugler@praetorian.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>

We want to use the C locale. Reported-by: Tobias Stoeckmann <tobias@stoeckmann.org> Signed-off-by: Alejandro Colomar <alx@kernel.org>

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

The APIs defined under each of those subdirs are too similar and related that it makes more sense to define them in the same files. (BTW, we only had one API per subdir, except in one subdir that had two APIs, so in the end, we have almost the same separation.) Signed-off-by: Alejandro Colomar <alx@kernel.org>

This also makes it consistent with strisdigit(). Signed-off-by: Alejandro Colomar <alx@kernel.org>

By being closer together, I find them more readable. The pattern and the differences are easier to spot. Signed-off-by: Alejandro Colomar <alx@kernel.org>

This makes it clear that it matches both C0 and C1 control characters. Signed-off-by: Alejandro Colomar <alx@kernel.org>

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

This compacts it into a one-liner, more similar to the strisascii_c() functions. Since we only use the argument once, we can even turn this into a macro. Signed-off-by: Alejandro Colomar <alx@kernel.org>

This allows us to not depend on whether strisprint_c("") returns true or false. Signed-off-by: Alejandro Colomar <alx@kernel.org>

It is not intuitive or clear what the right behavior should be for an empty string. If we define these APIs as "return true if all characters in the string belong to the specified character set", then an empty string should return true. On the other hand, if you ask me if an empty string is a numeric string, I might naively say no. It is irrelevant whether we return true or false for an empty string. All of the callers already handle correctly the case of an empty string. This makes the implementation simpler, using the argument only once. This allows implementing these as macros. Signed-off-by: Alejandro Colomar <alx@kernel.org>

Most of these are just one-liners, so they fit in less files, and that makes them even easier to compare all at once. Signed-off-by: Alejandro Colomar <alx@kernel.org>

This function returns true if the input character is a character from the POSIX portable filename character set. Link: <https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/V1_chap03.html#tag_03_265> Signed-off-by: Alejandro Colomar <alx@kernel.org>

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

In the first case, we can do the transformation because a few lines above, we explicitly reject a name starting with a '-'. In the second case, we're obviously using ispfchar_c() instead of its pattern. Signed-off-by: Alejandro Colomar <alx@kernel.org>

alejandro-colomar requested review from hallyn and ikerexxe January 1, 2026 00:40

alejandro-colomar self-assigned this Jan 1, 2026

alejandro-colomar force-pushed the chkname branch from f97b10f to 1ead5c6 Compare January 1, 2026 00:42

alejandro-colomar added the New API label Jan 1, 2026

alejandro-colomar force-pushed the chkname branch 2 times, most recently from 8fd9e48 to d39008c Compare January 2, 2026 00:59

stoeckmann suggested changes Jan 2, 2026

View reviewed changes

lib/string/ctype/isascii/ispfchar.h Outdated Show resolved Hide resolved

alejandro-colomar marked this pull request as draft January 2, 2026 19:35

alejandro-colomar force-pushed the chkname branch 3 times, most recently from e371b46 to 000c8c1 Compare January 3, 2026 16:56

alejandro-colomar requested a review from stoeckmann January 3, 2026 16:58

stoeckmann previously requested changes Jan 3, 2026

View reviewed changes

lib/chkname.c Show resolved Hide resolved

alejandro-colomar force-pushed the chkname branch from 000c8c1 to ed15a3c Compare February 23, 2026 14:39

alejandro-colomar force-pushed the chkname branch from ed15a3c to 42b2e85 Compare March 4, 2026 16:48

alejandro-colomar force-pushed the chkname branch from 42b2e85 to d3f1d69 Compare March 11, 2026 19:56

alejandro-colomar added 12 commits March 13, 2026 00:58

lib/string/ctype/isascii.h: iscntrl_c0c1(): Add macro

7dac526

This matches the two categories (C0, C1) of control characters. Link: <https://en.wikipedia.org/wiki/C0_and_C1_control_codes> Cc: KhaelK-Praetorian <khael.kugler@praetorian.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/, src/: Use isascii_c() functions instead of isascii(3)

a3f1f85

We want to use the C locale. Reported-by: Tobias Stoeckmann <tobias@stoeckmann.org> Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/: Use iscntrl_c0c1() instead of its pattern

b71857f

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

lib/string/ctype/strisascii.h: strisprint(): Simplify implementation

ba50c8d

This also makes it consistent with strisdigit(). Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/string/ctype/strisascii.h: Compact definitions

9d654d7

By being closer together, I find them more readable. The pattern and the differences are easier to spot. Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/: Rename strchriscntrl{ => _c0c1}()

03ee1a1

This makes it clear that it matches both C0 and C1 control characters. Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/: stris*(): Rename C-locale APIs with a _c suffix

4c41186

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

lib/string/ctype/strchrisascii.h: Use strpbrk(3) to simplify

e219989

This compacts it into a one-liner, more similar to the strisascii_c() functions. Since we only use the argument once, we can even turn this into a macro. Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/fields.c: valid_field(): Check empty string before strisprint_c()

71d183c

This allows us to not depend on whether strisprint_c("") returns true or false. Signed-off-by: Alejandro Colomar <alx@kernel.org>

alejandro-colomar added 5 commits March 13, 2026 01:19

lib/: Merge "lib/string/ctype/*" files even further

bee6bdf

Most of these are just one-liners, so they fit in less files, and that makes them even easier to compare all at once. Signed-off-by: Alejandro Colomar <alx@kernel.org>

lib/chkname.c: is_valid_name(): Use isalnum(3) instead of its pattern

08d72ce

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

lib/chkname.c: is_valid_name(): Split Samba check

82202d2

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

alejandro-colomar force-pushed the chkname branch from d3f1d69 to 4f120ee Compare March 13, 2026 00:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib/: Add ispfchar(), and use it instead of its pattern#1469

lib/: Add ispfchar(), and use it instead of its pattern#1469
alejandro-colomar wants to merge 17 commits intoshadow-maint:masterfrom
alejandro-colomar:chkname

alejandro-colomar commented Jan 1, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

alejandro-colomar commented Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alejandro-colomar commented Jan 1, 2026 •

edited

Loading