search

shadow-maint / shadow

Upstream shadow tree
Other
304 stars 232 forks source link

Simplify string handling #1048

Open alejandro-colomar opened 3 months ago

alejandro-colomar commented 3 months ago

Among other things, remove most uses of strtok(3). Replace its calls by strsep(3), which is stateless.

This probably silently fixes some bugs, where two separators were being merged as the same one.

Also, that's the reason why I didn't replace all strtok(3) calls. In some cases (paths), it makes sense to merge adjacent //.

Revisions:

v1b - Rebase ``` $ git range-diff master..gh/string shadow/master..string 1: 5ba4cfca ! 1: e825c03e lib/, src/: Use stpspn() instead of its pattern @@ lib/getdate.y: yylex (void) ## lib/limits.c ## @@ - #include "atoi/a2i.h" - #include "atoi/str2i.h" + #include "atoi/str2i/str2s.h" + #include "atoi/str2i/str2u.h" #include "memzero.h" +#include "string/strchr/stpspn.h" #include "typetraits.h" @@ lib/strtoday.c @@ #ident "$Id$" - #include "atoi/str2i.h" + #include "atoi/str2i/str2s.h" -#include "prototypes.h" #include "getdate.h" +#include "prototypes.h" 2: f5cde8a7 = 2: e7710fda lib/port.c: portcmp(): Use strcmp(3) instead of its pattern 3: 009a6aaa = 3: 13ff7c5a lib/gshadow.c: endsgent(): Remove dead assignment 4: 49317790 = 4: f090d6c3 src/suauth.c: check_su_auth(): Use strspn(3) instead of its pattern 5: 1e064044 = 5: 526c8f72 src/suauth.c: check_su_auth(): Use pointers to simplify 6: faae7f68 = 6: 30835d33 lib/, src/: Use strsep(3) instead of strtok(3) 7: edd0e442 = 7: e4213fdf lib/loginprompt.c: login_prompt(): Use strtcpy() instead of its pattern 8: 0cd40e34 = 8: 446541e1 contrib/, lib/, src/: Use strchr(3) to compact comparisons 9: c960c5f9 ! 9: 946d07ca lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons @@ lib/getrange.c #include +#include - #include "atoi/a2i.h" + #include "atoi/a2i/a2u.h" #include "defines.h" @@ lib/getrange.c: getrange(const char *range, return 0; /* */ @@ lib/strtoday.c -#ident "$Id$" +#include - #include "atoi/str2i.h" + #include "atoi/str2i/str2s.h" #include "getdate.h" @@ * 24-sep-72 10: fbbbd797 = 10: da0b2f80 contrib/, lib/, src/: Use consistent style using strchr(3) in conditionals 11: 812cbae4 = 11: 4d80b61b lib/fields.c: Remove dead code ```
v1c - Rebase ``` $ git range-diff master..gh/string shadow/master..string 1: e825c03e = 1: 9688e694 lib/, src/: Use stpspn() instead of its pattern 2: e7710fda = 2: 3ad3a4fc lib/port.c: portcmp(): Use strcmp(3) instead of its pattern 3: 13ff7c5a = 3: 35252105 lib/gshadow.c: endsgent(): Remove dead assignment 4: f090d6c3 = 4: 70bd2134 src/suauth.c: check_su_auth(): Use strspn(3) instead of its pattern 5: 526c8f72 = 5: b1d22c99 src/suauth.c: check_su_auth(): Use pointers to simplify 6: 30835d33 = 6: f78b0844 lib/, src/: Use strsep(3) instead of strtok(3) 7: e4213fdf = 7: 13b28353 lib/loginprompt.c: login_prompt(): Use strtcpy() instead of its pattern 8: 446541e1 = 8: 4df5efb7 contrib/, lib/, src/: Use strchr(3) to compact comparisons 9: 946d07ca ! 9: ff3588d5 lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons @@ Commit message ## lib/chkname.c ## @@ - #include #include #include + #include +#include + #include #include - #include "defines.h" @@ lib/chkname.c: login_name_max_size(void) } 10: da0b2f80 = 10: fa44b22f contrib/, lib/, src/: Use consistent style using strchr(3) in conditionals 11: 4d80b61b = 11: 1eb75b33 lib/fields.c: Remove dead code ```
v1d - Rebase ``` $ git range-diff alx/master..gh/string shadow/master..string 1: 9688e694 = 1: 7442a127 lib/, src/: Use stpspn() instead of its pattern 2: 3ad3a4fc = 2: 5a46723a lib/port.c: portcmp(): Use strcmp(3) instead of its pattern 3: 35252105 = 3: 93362cc3 lib/gshadow.c: endsgent(): Remove dead assignment 4: 70bd2134 = 4: ae1c284e src/suauth.c: check_su_auth(): Use strspn(3) instead of its pattern 5: b1d22c99 = 5: 376e4166 src/suauth.c: check_su_auth(): Use pointers to simplify 6: f78b0844 = 6: db93a222 lib/, src/: Use strsep(3) instead of strtok(3) 7: 13b28353 = 7: 9a6e3284 lib/loginprompt.c: login_prompt(): Use strtcpy() instead of its pattern 8: 4df5efb7 = 8: d3350b67 contrib/, lib/, src/: Use strchr(3) to compact comparisons 9: ff3588d5 = 9: 44df1083 lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons 10: fa44b22f = 10: b71d2236 contrib/, lib/, src/: Use consistent style using strchr(3) in conditionals 11: 1eb75b33 = 11: a719bc39 lib/fields.c: Remove dead code ```
v2 - Drop bogus commit. ``` $ git range-diff master gh/string string 1: 7442a127 = 1: 7442a127 lib/, src/: Use stpspn() instead of its pattern 2: 5a46723a = 2: 5a46723a lib/port.c: portcmp(): Use strcmp(3) instead of its pattern 3: 93362cc3 = 3: 93362cc3 lib/gshadow.c: endsgent(): Remove dead assignment 4: ae1c284e = 4: ae1c284e src/suauth.c: check_su_auth(): Use strspn(3) instead of its pattern 5: 376e4166 = 5: 376e4166 src/suauth.c: check_su_auth(): Use pointers to simplify 6: db93a222 = 6: db93a222 lib/, src/: Use strsep(3) instead of strtok(3) 7: 9a6e3284 = 7: 9a6e3284 lib/loginprompt.c: login_prompt(): Use strtcpy() instead of its pattern 8: d3350b67 < -: -------- contrib/, lib/, src/: Use strchr(3) to compact comparisons 9: 44df1083 ! 8: 34737968 lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons @@ lib/valid.c: bool valid (const char *password, const struct passwd *ent) * Now, perform the encryption using the salt from before on ## src/grpck.c ## -@@ src/grpck.c: check_grp_file(int *errors, bool *changed) +@@ src/grpck.c: static void check_grp_file (int *errors, bool *changed) */ if ( (NULL != grp->gr_mem[0]) && (NULL == grp->gr_mem[1]) @@ src/grpck.c: check_grp_file(int *errors, bool *changed) ## src/login.c ## -@@ src/login.c: main(int argc, char **argv) +@@ src/login.c: int main (int argc, char **argv) /* if we didn't get a user on the command line, set it to NULL */ get_pam_user (&pam_user); @@ src/login.c: main(int argc, char **argv) retcode = pam_set_item (pamh, PAM_USER, NULL); PAM_FAIL_CHECK; } -@@ src/login.c: main(int argc, char **argv) +@@ src/login.c: int main (int argc, char **argv) username = XMALLOC(max_size, char); login_prompt(username, max_size); @@ src/login.c: main(int argc, char **argv) /* Prompt for a new login */ free (username); username = NULL; -@@ src/login.c: main(int argc, char **argv) +@@ src/login.c: int main (int argc, char **argv) * guys won't see that the passwordless account exists at * all). --marekm */ 10: b71d2236 ! 9: 5f0b6d47 contrib/, lib/, src/: Use consistent style using strchr(3) in conditionals @@ Commit message Signed-off-by: Alejandro Colomar ## contrib/adduser.c ## -@@ contrib/adduser.c: main(void) +@@ contrib/adduser.c: main (void) printf ("That name is in use, choose another.\n"); done = 0; } 11: a719bc39 = 10: ad52f5f5 lib/fields.c: Remove dead code ```
v2b - Rebase ``` $ git range-diff alx/master..gh/string shadow/master..string 1: 7442a127 = 1: 6f5c3680 lib/, src/: Use stpspn() instead of its pattern 2: 5a46723a = 2: 7ce03828 lib/port.c: portcmp(): Use strcmp(3) instead of its pattern 3: 93362cc3 = 3: 0fcb014b lib/gshadow.c: endsgent(): Remove dead assignment 4: ae1c284e = 4: fb3d9fe0 src/suauth.c: check_su_auth(): Use strspn(3) instead of its pattern 5: 376e4166 = 5: db8c4239 src/suauth.c: check_su_auth(): Use pointers to simplify 6: db93a222 = 6: 79b5a0c4 lib/, src/: Use strsep(3) instead of strtok(3) 7: 9a6e3284 = 7: af97262f lib/loginprompt.c: login_prompt(): Use strtcpy() instead of its pattern 8: 34737968 = 8: 357e5e63 lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons 9: 5f0b6d47 = 9: dad01af5 contrib/, lib/, src/: Use consistent style using strchr(3) in conditionals 10: ad52f5f5 = 10: 5b35729d lib/fields.c: Remove dead code ```
v2c - Rebase ``` $ git range-diff master..gh/string shadow/master..string 1: 6f5c3680 = 1: 66270bd9 lib/, src/: Use stpspn() instead of its pattern 2: 7ce03828 = 2: 46e8763b lib/port.c: portcmp(): Use strcmp(3) instead of its pattern 3: 0fcb014b = 3: ab2551eb lib/gshadow.c: endsgent(): Remove dead assignment 4: fb3d9fe0 = 4: de8f691d src/suauth.c: check_su_auth(): Use strspn(3) instead of its pattern 5: db8c4239 = 5: 68792229 src/suauth.c: check_su_auth(): Use pointers to simplify 6: 79b5a0c4 = 6: ab0b9884 lib/, src/: Use strsep(3) instead of strtok(3) 7: af97262f = 7: 1a943f2c lib/loginprompt.c: login_prompt(): Use strtcpy() instead of its pattern 8: 357e5e63 = 8: 4e1c5add lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons 9: dad01af5 = 9: d2c77cf9 contrib/, lib/, src/: Use consistent style using strchr(3) in conditionals 10: 5b35729d = 10: d70eb4d2 lib/fields.c: Remove dead code ```
v2d - Rebase ``` $ git range-diff master..gh/string shadow/master..string 1: 66270bd9 ! 1: 93711361 lib/, src/: Use stpspn() instead of its pattern @@ lib/limits.c @@ #include "atoi/str2i/str2s.h" #include "atoi/str2i/str2u.h" - #include "memzero.h" + #include "string/memset/memzero.h" +#include "string/strchr/stpspn.h" #include "typetraits.h" @@ lib/limits.c: static int do_user_limits (const char *buf, const char *name) ## lib/loginprompt.c ## @@ #include "getdef.h" - #include "memzero.h" #include "prototypes.h" + #include "string/memset/memzero.h" +#include "string/strchr/stpspn.h" #include "string/strtok/stpsep.h" 2: 46e8763b = 2: b2476283 lib/port.c: portcmp(): Use strcmp(3) instead of its pattern 3: ab2551eb = 3: 887a7e26 lib/gshadow.c: endsgent(): Remove dead assignment 4: de8f691d = 4: e466fa72 src/suauth.c: check_su_auth(): Use strspn(3) instead of its pattern 5: 68792229 = 5: f8e2a2d0 src/suauth.c: check_su_auth(): Use pointers to simplify 6: ab0b9884 = 6: 3429f64f lib/, src/: Use strsep(3) instead of strtok(3) 7: 1a943f2c ! 7: 1a92729e lib/loginprompt.c: login_prompt(): Use strtcpy() instead of its pattern @@ Commit message ## lib/loginprompt.c ## @@ - #include "memzero.h" #include "prototypes.h" + #include "string/memset/memzero.h" #include "string/strchr/stpspn.h" +#include "string/strcpy/strtcpy.h" #include "string/strtok/stpsep.h" 8: 4e1c5add = 8: 334095fd lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons 9: d2c77cf9 ! 9: e1180ed3 contrib/, lib/, src/: Use consistent style using strchr(3) in conditionals @@ lib/obscure.c +#include #include "attr.h" - #include "memzero.h" + #include "prototypes.h" @@ lib/obscure.c: static bool palindrome (MAYBE_UNUSED const char *old, const char *new) * more than half of the characters are different ones. */ 10: d70eb4d2 = 10: ffdf778d lib/fields.c: Remove dead code ```
v2e - Rebase ``` $ git range-diff gh/master..gh/string shadow/master..string 1: 93711361 = 1: 75c094da lib/, src/: Use stpspn() instead of its pattern 2: b2476283 = 2: 0a599855 lib/port.c: portcmp(): Use strcmp(3) instead of its pattern 3: 887a7e26 = 3: 8cbfdc6f lib/gshadow.c: endsgent(): Remove dead assignment 4: e466fa72 = 4: 76045e06 src/suauth.c: check_su_auth(): Use strspn(3) instead of its pattern 5: f8e2a2d0 = 5: 5e0c907a src/suauth.c: check_su_auth(): Use pointers to simplify 6: 3429f64f = 6: e7be1ce8 lib/, src/: Use strsep(3) instead of strtok(3) 7: 1a92729e = 7: e7dc5f26 lib/loginprompt.c: login_prompt(): Use strtcpy() instead of its pattern 8: 334095fd ! 8: a2c70df5 lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons @@ lib/chkname.c #include #include -@@ lib/chkname.c: login_name_max_size(void) - } - - --static bool is_valid_name (const char *name) -+static bool -+is_valid_name(const char *name) - { - if (allow_bad_names) { - return true; -@@ lib/chkname.c: static bool is_valid_name (const char *name) +@@ lib/chkname.c: is_valid_name(const char *name) */ int numeric; @@ lib/chkname.c: static bool is_valid_name (const char *name) !((*name >= 'a' && *name <= 'z') || (*name >= 'A' && *name <= 'Z') || (*name >= '0' && *name <= '9') || -@@ lib/chkname.c: static bool is_valid_name (const char *name) +@@ lib/chkname.c: is_valid_name(const char *name) *name == '_' || *name == '.' || *name == '-' || - (*name == '$' && name[1] == '\0') + strcmp(name, "$") == 0 - )) { - return false; - } + )) + { + errno = EINVAL; ## lib/getrange.c ## @@ 9: e1180ed3 = 9: 9af0b281 contrib/, lib/, src/: Use consistent style using strchr(3) in conditionals 10: ffdf778d = 10: afd0b323 lib/fields.c: Remove dead code ```
v3 - Fix mistake. ``` $ git range-diff str gh/string string 1: e7be1ce8 = 1: e7be1ce8 lib/, src/: Use strsep(3) instead of strtok(3) 2: e7dc5f26 = 2: e7dc5f26 lib/loginprompt.c: login_prompt(): Use strtcpy() instead of its pattern 3: a2c70df5 ! 3: 25059777 lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons @@ lib/port.c: next: */ - for (j = 0; ('\0' != *cp) && (j < PORT_TIMES); j++) { -+ for (j = 0; strcmp(cp, "") == 0 && (j < PORT_TIMES); j++) { ++ for (j = 0; strcmp(cp, "") != 0 && (j < PORT_TIMES); j++) { /* * Start off with no days of the week 4: 9af0b281 = 4: 8412e015 contrib/, lib/, src/: Use consistent style using strchr(3) in conditionals 5: afd0b323 = 5: b6191b16 lib/fields.c: Remove dead code ```
v3b - Rebase ``` $ git range-diff gh/str..gh/string master..string 1: e7be1ce8 = 1: 60d9fa84 lib/, src/: Use strsep(3) instead of strtok(3) 2: e7dc5f26 = 2: 4562e89d lib/loginprompt.c: login_prompt(): Use strtcpy() instead of its pattern 3: 25059777 = 3: 9fcea456 lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons 4: 8412e015 = 4: 8e2a3591 contrib/, lib/, src/: Use consistent style using strchr(3) in conditionals 5: b6191b16 = 5: be7b3022 lib/fields.c: Remove dead code ```
v3c - Reorder commits ``` $ git range-diff master gh/string string 2: 4562e89d = 1: 3a253dc1 lib/loginprompt.c: login_prompt(): Use strtcpy() instead of its pattern 1: 60d9fa84 = 2: 1ada0038 lib/, src/: Use strsep(3) instead of strtok(3) 3: 9fcea456 = 3: 874c025e lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons 4: 8e2a3591 = 4: 0f3baf67 contrib/, lib/, src/: Use consistent style using strchr(3) in conditionals 5: be7b3022 = 5: a4838e69 lib/fields.c: Remove dead code ```
v3d - Rebase (after the 2 PRs that contain some of the commits in this one). ``` $ git range-diff master..gh/string strsep..string 1: 3a253dc1 < -: -------- lib/loginprompt.c: login_prompt(): Use strtcpy() instead of its pattern 2: 1ada0038 < -: -------- lib/, src/: Use strsep(3) instead of strtok(3) 3: 874c025e = 1: ec607650 lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons 4: 0f3baf67 = 2: c81a9322 contrib/, lib/, src/: Use consistent style using strchr(3) in conditionals 5: a4838e69 = 3: 5f396feb lib/fields.c: Remove dead code ```
v3e - Rebase ``` $ git range-diff gh/strsep..gh/string strsep..string 1: ec607650 = 1: ee5bceb6 lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons 2: c81a9322 = 2: 2f753cf6 contrib/, lib/, src/: Use consistent style using strchr(3) in conditionals 3: 5f396feb = 3: aa3f0f9d lib/fields.c: Remove dead code ```
alejandro-colomar commented 2 months ago

Do NOT merge, please; I've found issues in this branch in chage(1). I'll debug it.

$ sudo ./src/chage -d 2023-09-20 alx
chage: invalid date '2023-09-20'

Edit: Fixed in v2. strchr(2) cannot replace byte comparisons, because a '\0' is a match.

alejandro-colomar commented 2 weeks ago

@hallyn would you need me to split this PR into smaller chunks for making it easier for you to review? Or any other change you'd like to see?

Maybe strtok=>strsep is a large change, less trivial than the previous ones, and I should split right before the strtok changes.

alejandro-colomar commented 2 weeks ago

Let's queue this after https://github.com/shadow-maint/shadow/pull/1085