shadow-maint / shadow

Upstream shadow tree
Other
306 stars 237 forks source link

Replace most strtok(3) calls by strsep(3) #1093

Open alejandro-colomar opened 1 month ago

alejandro-colomar commented 1 month ago

This is a subset of #1048 for easier review.


Revisions:

v1b - Rebase ``` $ git range-diff gh/strtcpy..gh/strsep strtcpy..strsep 1: 1ada0038 = 1: b806041d lib/, src/: Use strsep(3) instead of strtok(3) ```
v1c - Rebase ``` $ git range-diff gh/strtcpy..gh/strsep master..strsep 1: b806041d = 1: b87f79c6 lib/, src/: Use strsep(3) instead of strtok(3) ```
v1d - Expand commit message ``` $ git range-diff master gh/strsep strsep 1: b87f79c6 ! 1: 75903c36 lib/, src/: Use strsep(3) instead of strtok(3) @@ Metadata ## Commit message ## lib/, src/: Use strsep(3) instead of strtok(3) + strsep(3) is stateless, and so is easier to reason about. + + It also has a slight difference: strtok(3) jumps over empty fields, + while strsep(3) respects them as empty fields. In most of the cases + where we were using strtok(3), it makes more sense to respect empty + fields, and this commit probably silently fixes a few bugs. + + In other cases (most notably filesystem paths), contiguous delimiters + ("//") should be collapsed, so strtok(3) still makes more sense there. + This commit doesn't replace such strtok(3) calls. + Signed-off-by: Alejandro Colomar ## lib/addgrps.c ## ```
v1e - Rebase ``` $ git range-diff master..gh/strsep shadow/master..strsep 1: 75903c36 = 1: fa952752 lib/, src/: Use strsep(3) instead of strtok(3) ```
v2 - Several rebases ``` $ git range-diff fa952752^..fa952752 shadow/master..gh/strsep 1: fa952752 = 1: eb9e6257 lib/, src/: Use strsep(3) instead of strtok(3) ```
v2b - Rebase ``` $ git range-diff alx/master..gh/strsep shadow/master..strsep 1: eb9e6257 = 1: 7f42b741 lib/, src/: Use strsep(3) instead of strtok(3) ```
v2c - Rebase ``` $ git range-diff gh/master..gh/strsep shadow/master..strsep 1: 7f42b741 ! 1: 40a468ab lib/, src/: Use strsep(3) instead of strtok(3) @@ src/login_nopam.c: static bool list_match (char *list, const char *item, bool (* + while ( (NULL != (tok = strsep(&list, ", \t"))) && (strcasecmp (tok, "EXCEPT") != 0)) /* VOID */ ; - if (tok == 0 || !list_match (NULL, item, match_fn)) { + if (tok == NULL || !list_match(NULL, item, match_fn)) { ## src/suauth.c ## @@ src/suauth.c: static int isgrp (const char *, const char *); ```
alejandro-colomar commented 2 weeks ago

@hallyn

This is a little bit of a bottleneck to me (so many local patches that depend on this are waiting for publication). Would you mind prioritizing review of this over my other patch sets? :-)

alejandro-colomar commented 2 weeks ago

BTW, please be very careful/skeptic. If you know how to test some of these changes, it would be good to test them. The change from strtok(3) to strsep(3) has small implications, and we should be especially careful.