Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add strisdigit(), and use it instead of its pattern #1153

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alejandro-colomar
Copy link
Collaborator

@alejandro-colomar alejandro-colomar commented Dec 10, 2024


Revisions:

v2
  • Add strisdigit() and use it instead of open-coding it.
  • Add note about empty strings in commit message.
$ git range-diff master gh/alldigits alldigits 
-:  -------- > 1:  40c9035b lib/string/ctype/: strisdigit(): Add function
1:  a349dfd3 ! 2:  24335081 lib/: Reimplement all_digits() in terms of streq(stpspn()), and open-code it
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/: Reimplement all_digits() in terms of streq(stpspn()), and open-code it
    +    lib/: Use strisdigit() instead of its pattern
    +
    +    Note that the old code in
    +
    +            (1)  lib/strtoday.c:strtoday()
    +            (2)  lib/subordinateio.c:append_uids()
    +
    +    was considering an empty string as if it were a number.
    +    strisdigit() does not consider an empty string to be numeric.
    +
    +    I think it will not affect the behavior in either case, as they should
    +    sooner or later result in an error somewhere.  And it seems (IMO)
    +    surprising to treat empty strings as numeric strings, so let's not do
    +    it.
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    @@ lib/chkname.c
      
      #include "defines.h"
      #include "chkname.h"
    -+#include "string/strchr/stpspn.h"
    ++#include "string/ctype/strisdigit.h"
      #include "string/strcmp/streq.h"
      
      
    @@ lib/chkname.c: is_valid_name(const char *name)
               */
     -  int numeric;
     +
    -+  if (streq(stpspn(name, "0123456789"), "")) {
    ++  if (strisdigit(name)) {
     +          errno = EINVAL;
     +          return false;
     +  }
    @@ lib/chkname.c: is_valid_name(const char *name)
        return true;
     
      ## lib/strtoday.c ##
    +@@
    + #include "atoi/str2i/str2s.h"
    + #include "getdate.h"
    + #include "prototypes.h"
    ++#include "string/ctype/strisdigit.h"
    + #include "string/strchr/stpspn.h"
    + #include "string/strcmp/streq.h"
    + 
     @@
      long strtoday (const char *str)
      {
    @@ lib/strtoday.c: long strtoday (const char *str)
     -          s++;
     -  }
     -  if (isnum) {
    -+  if (streq(stpspn(s, "0123456789"), "")) {
    ++  if (strisdigit(s)) {
                long retdate;
     +
                if (str2sl(&retdate, str) == -1)
    @@ lib/strtoday.c: long strtoday (const char *str)
     
      ## lib/subordinateio.c ##
     @@
    + #include "alloc/realloc.h"
      #include "alloc/reallocf.h"
      #include "atoi/str2i/str2u.h"
    ++#include "string/ctype/strisdigit.h"
      #include "string/sprintf/snprintf.h"
    -+#include "string/strchr/stpspn.h"
      #include "string/strcmp/streq.h"
      
    - 
     @@ lib/subordinateio.c: out:
        return count;
      }
    @@ lib/subordinateio.c: out:
        uid_t  owner_uid;
      
     -  if (all_digits(owner)) {
    -+  if (streq(stpspn(owner, "0123456789"), "")) {
    ++  if (strisdigit(owner)) {
                i = sscanf(owner, "%d", &owner_uid);
                if (i != 1) {
                        // should not happen
v3
  • Clarify that this API works with the C locale.
$ git range-diff master gh/alldigits alldigits 
1:  40c9035b ! 1:  6183a653 lib/string/ctype/: strisdigit(): Add function
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/string/ctype/: strisdigit(): Add function
    +    lib/string/ctype/: strisdigit_c(): Add function
    +
    +    The "_c" clarifies that is considers the digits from the C locale.
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    @@ lib/Makefile.am: libshadow_la_SOURCES = \
        spawn.c \
        sssd.c \
        sssd.h \
    -+  string/ctype/strisdigit.c \
    -+  string/ctype/strisdigit.h \
    ++  string/ctype/strisdigit_c.c \
    ++  string/ctype/strisdigit_c.h \
        string/memset/memzero.c \
        string/memset/memzero.h \
        string/sprintf/snprintf.c \
     
    - ## lib/string/ctype/strisdigit.c (new) ##
    + ## lib/string/ctype/strisdigit_c.c (new) ##
     @@
     +// SPDX-FileCopyrightText: 2024, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
    @@ lib/string/ctype/strisdigit.c (new)
     +
     +#include <config.h>
     +
    -+#include "string/ctype/strisdigit.h"
    ++#include "string/ctype/strisdigit_c.h"
     +
     +#include <stdbool.h>
     +
     +
    -+extern inline bool strisdigit(const char *s);
    ++extern inline bool strisdigit_c(const char *s);
     
    - ## lib/string/ctype/strisdigit.h (new) ##
    + ## lib/string/ctype/strisdigit_c.h (new) ##
     @@
     +// SPDX-FileCopyrightText: 2024, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
     +
     +
    -+#ifndef SHADOW_INCLUDE_LIB_STRING_CTYPE_STRISDIGIT_H_
    -+#define SHADOW_INCLUDE_LIB_STRING_CTYPE_STRISDIGIT_H_
    ++#ifndef SHADOW_INCLUDE_LIB_STRING_CTYPE_STRISDIGIT_C_H_
    ++#define SHADOW_INCLUDE_LIB_STRING_CTYPE_STRISDIGIT_C_H_
     +
     +
     +#include <config.h>
    @@ lib/string/ctype/strisdigit.h (new)
     +#include "string/strcmp/streq.h"
     +
     +
    -+inline bool strisdigit(const char *s);
    ++inline bool strisdigit_c(const char *s);
     +
     +
    -+// Like isdigit(3), but check all characters in the string
    ++// Like isdigit(3),
    ++// but check all characters in the string, and use the C locale.
     +inline bool
    -+strisdigit(const char *s)
    ++strisdigit_c(const char *s)
     +{
     +  if (streq(s, ""))
     +          return false;
2:  24335081 ! 2:  b9da40df lib/: Use strisdigit() instead of its pattern
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/: Use strisdigit() instead of its pattern
    +    lib/: Use strisdigit_c() instead of its pattern
     
         Note that the old code in
     
    @@ Commit message
                 (2)  lib/subordinateio.c:append_uids()
     
         was considering an empty string as if it were a number.
    -    strisdigit() does not consider an empty string to be numeric.
    +    strisdigit_c() does not consider an empty string to be numeric.
     
         I think it will not affect the behavior in either case, as they should
         sooner or later result in an error somewhere.  And it seems (IMO)
    @@ lib/chkname.c
      
      #include "defines.h"
      #include "chkname.h"
    -+#include "string/ctype/strisdigit.h"
    ++#include "string/ctype/strisdigit_c.h"
      #include "string/strcmp/streq.h"
      
      
    @@ lib/chkname.c: is_valid_name(const char *name)
               */
     -  int numeric;
     +
    -+  if (strisdigit(name)) {
    ++  if (strisdigit_c(name)) {
     +          errno = EINVAL;
     +          return false;
     +  }
    @@ lib/strtoday.c
      #include "atoi/str2i/str2s.h"
      #include "getdate.h"
      #include "prototypes.h"
    -+#include "string/ctype/strisdigit.h"
    ++#include "string/ctype/strisdigit_c.h"
      #include "string/strchr/stpspn.h"
      #include "string/strcmp/streq.h"
      
    @@ lib/strtoday.c: long strtoday (const char *str)
     -          s++;
     -  }
     -  if (isnum) {
    -+  if (strisdigit(s)) {
    ++  if (strisdigit_c(s)) {
                long retdate;
     +
                if (str2sl(&retdate, str) == -1)
    @@ lib/subordinateio.c
      #include "alloc/realloc.h"
      #include "alloc/reallocf.h"
      #include "atoi/str2i/str2u.h"
    -+#include "string/ctype/strisdigit.h"
    ++#include "string/ctype/strisdigit_c.h"
      #include "string/sprintf/snprintf.h"
      #include "string/strcmp/streq.h"
      
    @@ lib/subordinateio.c: out:
        uid_t  owner_uid;
      
     -  if (all_digits(owner)) {
    -+  if (strisdigit(owner)) {
    ++  if (strisdigit_c(owner)) {
                i = sscanf(owner, "%d", &owner_uid);
                if (i != 1) {
                        // should not happen
v4
  • Move files into subdirectory.
$ git range-diff master gh/alldigits alldigits 
1:  6183a653 ! 1:  e25769be lib/string/ctype/: strisdigit_c(): Add function
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/string/ctype/: strisdigit_c(): Add function
    +    lib/string/ctype/strisascii_c/: strisdigit_c(): Add function
     
    -    The "_c" clarifies that is considers the digits from the C locale.
    +    The "_c" means that it considers the digits from the C locale.
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    @@ lib/Makefile.am: libshadow_la_SOURCES = \
        spawn.c \
        sssd.c \
        sssd.h \
    -+  string/ctype/strisdigit_c.c \
    -+  string/ctype/strisdigit_c.h \
    ++  string/ctype/strisascii_c/strisdigit_c.c \
    ++  string/ctype/strisascii_c/strisdigit_c.h \
        string/memset/memzero.c \
        string/memset/memzero.h \
        string/sprintf/snprintf.c \
     
    - ## lib/string/ctype/strisdigit_c.c (new) ##
    + ## lib/string/ctype/strisascii_c/strisdigit_c.c (new) ##
     @@
     +// SPDX-FileCopyrightText: 2024, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
    @@ lib/string/ctype/strisdigit_c.c (new)
     +
     +extern inline bool strisdigit_c(const char *s);
     
    - ## lib/string/ctype/strisdigit_c.h (new) ##
    + ## lib/string/ctype/strisascii_c/strisdigit_c.h (new) ##
     @@
     +// SPDX-FileCopyrightText: 2024, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
2:  b9da40df ! 2:  418486c1 lib/: Use strisdigit_c() instead of its pattern
    @@ lib/chkname.c
      
      #include "defines.h"
      #include "chkname.h"
    -+#include "string/ctype/strisdigit_c.h"
    ++#include "string/ctype/strisascii_c/strisdigit_c.h"
      #include "string/strcmp/streq.h"
      
      
    @@ lib/strtoday.c
      #include "atoi/str2i/str2s.h"
      #include "getdate.h"
      #include "prototypes.h"
    -+#include "string/ctype/strisdigit_c.h"
    ++#include "string/ctype/strisascii_c/strisdigit_c.h"
      #include "string/strchr/stpspn.h"
      #include "string/strcmp/streq.h"
      
    @@ lib/subordinateio.c
      #include "alloc/realloc.h"
      #include "alloc/reallocf.h"
      #include "atoi/str2i/str2u.h"
    -+#include "string/ctype/strisdigit_c.h"
    ++#include "string/ctype/strisascii_c/strisdigit_c.h"
      #include "string/sprintf/snprintf.h"
      #include "string/strcmp/streq.h"
      
v4b
  • Fix include guard and include path.
$ git range-diff master gh/alldigits alldigits 
1:  e25769be ! 1:  d160bacc lib/string/ctype/strisascii_c/: strisdigit_c(): Add function
    @@ lib/string/ctype/strisascii_c/strisdigit_c.c (new)
     +
     +#include <config.h>
     +
    -+#include "string/ctype/strisdigit_c.h"
    ++#include "string/ctype/strisascii_c/strisdigit_c.h"
     +
     +#include <stdbool.h>
     +
    @@ lib/string/ctype/strisascii_c/strisdigit_c.h (new)
     +// SPDX-License-Identifier: BSD-3-Clause
     +
     +
    -+#ifndef SHADOW_INCLUDE_LIB_STRING_CTYPE_STRISDIGIT_C_H_
    -+#define SHADOW_INCLUDE_LIB_STRING_CTYPE_STRISDIGIT_C_H_
    ++#ifndef SHADOW_INCLUDE_LIB_STRING_CTYPE_STRISASCII_C_STRISDIGIT_C_H_
    ++#define SHADOW_INCLUDE_LIB_STRING_CTYPE_STRISASCII_C_STRISDIGIT_C_H_
     +
     +
     +#include <config.h>
2:  418486c1 = 2:  04c1c1b1 lib/: Use strisdigit_c() instead of its pattern
v5
  • Simplify name (character encodings are a mess, anyway).
$ git range-diff master gh/alldigits alldigits 
1:  d160bacc ! 1:  d5fb6c45 lib/string/ctype/strisascii_c/: strisdigit_c(): Add function
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/string/ctype/strisascii_c/: strisdigit_c(): Add function
    -
    -    The "_c" means that it considers the digits from the C locale.
    +    lib/string/ctype/strisascii/: strisdigit(): Add function
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    @@ lib/Makefile.am: libshadow_la_SOURCES = \
        spawn.c \
        sssd.c \
        sssd.h \
    -+  string/ctype/strisascii_c/strisdigit_c.c \
    -+  string/ctype/strisascii_c/strisdigit_c.h \
    ++  string/ctype/strisascii/strisdigit.c \
    ++  string/ctype/strisascii/strisdigit.h \
        string/memset/memzero.c \
        string/memset/memzero.h \
        string/sprintf/snprintf.c \
     
    - ## lib/string/ctype/strisascii_c/strisdigit_c.c (new) ##
    + ## lib/string/ctype/strisascii/strisdigit.c (new) ##
     @@
     +// SPDX-FileCopyrightText: 2024, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
    @@ lib/string/ctype/strisascii_c/strisdigit_c.c (new)
     +
     +#include <config.h>
     +
    -+#include "string/ctype/strisascii_c/strisdigit_c.h"
    ++#include "string/ctype/strisascii/strisdigit.h"
     +
     +#include <stdbool.h>
     +
     +
    -+extern inline bool strisdigit_c(const char *s);
    ++extern inline bool strisdigit(const char *s);
     
    - ## lib/string/ctype/strisascii_c/strisdigit_c.h (new) ##
    + ## lib/string/ctype/strisascii/strisdigit.h (new) ##
     @@
     +// SPDX-FileCopyrightText: 2024, Alejandro Colomar <alx@kernel.org>
     +// SPDX-License-Identifier: BSD-3-Clause
     +
     +
    -+#ifndef SHADOW_INCLUDE_LIB_STRING_CTYPE_STRISASCII_C_STRISDIGIT_C_H_
    -+#define SHADOW_INCLUDE_LIB_STRING_CTYPE_STRISASCII_C_STRISDIGIT_C_H_
    ++#ifndef SHADOW_INCLUDE_LIB_STRING_CTYPE_STRISASCII_STRISDIGIT_H_
    ++#define SHADOW_INCLUDE_LIB_STRING_CTYPE_STRISASCII_STRISDIGIT_H_
     +
     +
     +#include <config.h>
    @@ lib/string/ctype/strisascii_c/strisdigit_c.h (new)
     +#include "string/strcmp/streq.h"
     +
     +
    -+inline bool strisdigit_c(const char *s);
    ++inline bool strisdigit(const char *s);
     +
     +
    -+// Like isdigit(3),
    -+// but check all characters in the string, and use the C locale.
    ++// Like isdigit(3), but check all characters in the string.
     +inline bool
    -+strisdigit_c(const char *s)
    ++strisdigit(const char *s)
     +{
     +  if (streq(s, ""))
     +          return false;
2:  04c1c1b1 ! 2:  e22bf379 lib/: Use strisdigit_c() instead of its pattern
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/: Use strisdigit_c() instead of its pattern
    +    lib/: Use strisdigit() instead of its pattern
     
         Note that the old code in
     
    @@ Commit message
                 (2)  lib/subordinateio.c:append_uids()
     
         was considering an empty string as if it were a number.
    -    strisdigit_c() does not consider an empty string to be numeric.
    +    strisdigit() does not consider an empty string to be numeric.
     
         I think it will not affect the behavior in either case, as they should
         sooner or later result in an error somewhere.  And it seems (IMO)
    @@ lib/chkname.c
      
      #include "defines.h"
      #include "chkname.h"
    -+#include "string/ctype/strisascii_c/strisdigit_c.h"
    ++#include "string/ctype/strisascii/strisdigit.h"
      #include "string/strcmp/streq.h"
      
      
    @@ lib/chkname.c: is_valid_name(const char *name)
               */
     -  int numeric;
     +
    -+  if (strisdigit_c(name)) {
    ++  if (strisdigit(name)) {
     +          errno = EINVAL;
     +          return false;
     +  }
    @@ lib/strtoday.c
      #include "atoi/str2i/str2s.h"
      #include "getdate.h"
      #include "prototypes.h"
    -+#include "string/ctype/strisascii_c/strisdigit_c.h"
    ++#include "string/ctype/strisascii/strisdigit.h"
      #include "string/strchr/stpspn.h"
      #include "string/strcmp/streq.h"
      
    @@ lib/strtoday.c: long strtoday (const char *str)
     -          s++;
     -  }
     -  if (isnum) {
    -+  if (strisdigit_c(s)) {
    ++  if (strisdigit(s)) {
                long retdate;
     +
                if (str2sl(&retdate, str) == -1)
    @@ lib/subordinateio.c
      #include "alloc/realloc.h"
      #include "alloc/reallocf.h"
      #include "atoi/str2i/str2u.h"
    -+#include "string/ctype/strisascii_c/strisdigit_c.h"
    ++#include "string/ctype/strisascii/strisdigit.h"
      #include "string/sprintf/snprintf.h"
      #include "string/strcmp/streq.h"
      
    @@ lib/subordinateio.c: out:
        uid_t  owner_uid;
      
     -  if (all_digits(owner)) {
    -+  if (strisdigit_c(owner)) {
    ++  if (strisdigit(owner)) {
                i = sscanf(owner, "%d", &owner_uid);
                if (i != 1) {
                        // should not happen

@alejandro-colomar alejandro-colomar force-pushed the alldigits branch 3 times, most recently from a349dfd to 2433508 Compare December 11, 2024 01:23
@alejandro-colomar alejandro-colomar changed the title lib/subordinateio.c: Reimplement all_digits() in terms of streq(stpspn()), and open-code it lib/string/ctype/, lib/: Add and use strisdigit() Dec 11, 2024
@alejandro-colomar alejandro-colomar changed the title lib/string/ctype/, lib/: Add and use strisdigit() Add strisdigit(), and use it instead of its pattern Dec 11, 2024
@alejandro-colomar alejandro-colomar force-pushed the alldigits branch 3 times, most recently from 418486c to 04c1c1b Compare December 13, 2024 02:06
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Note that the old code in

	(1)  lib/strtoday.c:strtoday()
	(2)  lib/subordinateio.c:append_uids()

was considering an empty string as if it were a number.
strisdigit() does not consider an empty string to be numeric.

I think it will not affect the behavior in either case, as they should
sooner or later result in an error somewhere.  And it seems (IMO)
surprising to treat empty strings as numeric strings, so let's not do
it.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar alejandro-colomar marked this pull request as ready for review December 22, 2024 11:56
@alejandro-colomar alejandro-colomar marked this pull request as draft December 24, 2024 17:40
@alejandro-colomar alejandro-colomar marked this pull request as ready for review December 24, 2024 17:43
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.

1 participant