From 90afe61003efd885df252764aef2a31c185fe55f Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Thu, 4 Jul 2024 13:21:12 +0200 Subject: [PATCH] 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. While at this, remove some useless variables used by these calls, and reduce the scope of others. Signed-off-by: Alejandro Colomar --- lib/addgrps.c | 21 +++++++++++---------- lib/console.c | 10 +++++----- lib/motd.c | 12 +++++------- src/groupadd.c | 21 ++++++++++++--------- src/groupmod.c | 20 +++++++++++--------- src/login_nopam.c | 30 +++++++++++++++++------------- src/suauth.c | 21 +++++++++------------ 7 files changed, 70 insertions(+), 65 deletions(-) diff --git a/lib/addgrps.c b/lib/addgrps.c index b2e17a9d..97c47e07 100644 --- a/lib/addgrps.c +++ b/lib/addgrps.c @@ -14,9 +14,10 @@ #include "prototypes.h" #include "defines.h" -#include -#include #include +#include +#include +#include #include "alloc/malloc.h" #include "alloc/reallocf.h" @@ -24,19 +25,19 @@ #ident "$Id$" -#define SEP ",:" /* * Add groups with names from LIST (separated by commas or colons) * to the supplementary group set. Silently ignore groups which are - * already there. Warning: uses strtok(). + * already there. */ -int add_groups (const char *list) +int +add_groups(const char *list) { GETGROUPS_T *grouplist; size_t i; int ngroups; bool added; - char *token; + char *g, *p; char buf[1024]; int ret; FILE *shadow_logfd = log_get_logfd(); @@ -71,13 +72,13 @@ int add_groups (const char *list) } added = false; - for (token = strtok (buf, SEP); NULL != token; token = strtok (NULL, SEP)) { + p = buf; + while (NULL != (g = strsep(&p, ",:"))) { struct group *grp; - grp = getgrnam (token); /* local, no need for xgetgrnam */ + grp = getgrnam(g); /* local, no need for xgetgrnam */ if (NULL == grp) { - fprintf (shadow_logfd, _("Warning: unknown group %s\n"), - token); + fprintf(shadow_logfd, _("Warning: unknown group %s\n"), g); continue; } diff --git a/lib/console.c b/lib/console.c index b9f088d2..28389331 100644 --- a/lib/console.c +++ b/lib/console.c @@ -26,7 +26,8 @@ * under "cfgin" in config (directly or indirectly). Fallback to default if * something is bad. */ -static bool is_listed (const char *cfgin, const char *tty, bool def) +static bool +is_listed(const char *cfgin, const char *tty, bool def) { FILE *fp; char buf[1024], *s; @@ -49,14 +50,13 @@ static bool is_listed (const char *cfgin, const char *tty, bool def) if (*cons != '/') { char *pbuf; + STRTCPY(buf, cons); - pbuf = &buf[0]; - while ((s = strtok (pbuf, ":")) != NULL) { + pbuf = buf; + while (NULL != (s = strsep(&pbuf, ":"))) { if (streq(s, tty)) { return true; } - - pbuf = NULL; } return false; } diff --git a/lib/motd.c b/lib/motd.c index 52712675..6394dbd9 100644 --- a/lib/motd.c +++ b/lib/motd.c @@ -12,6 +12,7 @@ #ident "$Id$" #include +#include #include "defines.h" #include "getdef.h" @@ -26,7 +27,8 @@ * it to the user's terminal at login time. The MOTD_FILE configuration * option is a colon-delimited list of filenames. */ -void motd (void) +void +motd(void) { FILE *fp; char *motdlist; @@ -41,12 +43,8 @@ void motd (void) motdlist = xstrdup (motdfile); - for (mb = motdlist; ;mb = NULL) { - motdfile = strtok (mb, ":"); - if (NULL == motdfile) { - break; - } - + mb = motdlist; + while (NULL != (motdfile = strsep(&mb, ":"))) { fp = fopen (motdfile, "r"); if (NULL != fp) { while ((c = getc (fp)) != EOF) { diff --git a/src/groupadd.c b/src/groupadd.c index 263dc93a..9f0eb2e5 100644 --- a/src/groupadd.c +++ b/src/groupadd.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #ifdef ACCT_TOOLS_SETUID #ifdef USE_PAM @@ -164,7 +165,8 @@ static void new_sgent (struct sgrp *sgent) * * grp_update() writes the new records to the group files. */ -static void grp_update (void) +static void +grp_update(void) { struct group grp; @@ -196,19 +198,20 @@ static void grp_update (void) #endif /* SHADOWGRP */ if (user_list) { - char *token; - token = strtok(user_list, ","); - while (token) { - if (prefix_getpwnam (token) == NULL) { - fprintf (stderr, _("Invalid member username %s\n"), token); + char *u, *ul; + + ul = user_list; + while (NULL != (u = strsep(&ul, ","))) { + if (prefix_getpwnam(u) == NULL) { + fprintf(stderr, _("Invalid member username %s\n"), u); exit (E_GRP_UPDATE); } - grp.gr_mem = add_list(grp.gr_mem, token); + + grp.gr_mem = add_list(grp.gr_mem, u); #ifdef SHADOWGRP if (is_shadow_grp) - sgrp.sg_mem = add_list(sgrp.sg_mem, token); + sgrp.sg_mem = add_list(sgrp.sg_mem, u); #endif - token = strtok(NULL, ","); } } diff --git a/src/groupmod.c b/src/groupmod.c index ee1b4a9d..7342707d 100644 --- a/src/groupmod.c +++ b/src/groupmod.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #ifdef ACCT_TOOLS_SETUID @@ -198,7 +199,8 @@ static void new_sgent (struct sgrp *sgent) * * grp_update() updates the new records in the memory databases. */ -static void grp_update (void) +static void +grp_update(void) { struct group grp; const struct group *ogrp; @@ -251,7 +253,7 @@ static void grp_update (void) } if (user_list) { - char *token; + char *u, *ul; if (!aflg) { // requested to replace the existing groups @@ -274,18 +276,18 @@ static void grp_update (void) } #endif /* SHADOWGRP */ - token = strtok(user_list, ","); - while (token) { - if (prefix_getpwnam (token) == NULL) { - fprintf (stderr, _("Invalid member username %s\n"), token); + ul = user_list; + while (NULL != (u = strsep(&ul, ","))) { + if (prefix_getpwnam(u) == NULL) { + fprintf(stderr, _("Invalid member username %s\n"), u); exit (E_GRP_UPDATE); } - grp.gr_mem = add_list(grp.gr_mem, token); + + grp.gr_mem = add_list(grp.gr_mem, u); #ifdef SHADOWGRP if (NULL != osgrp) - sgrp.sg_mem = add_list(sgrp.sg_mem, token); + sgrp.sg_mem = add_list(sgrp.sg_mem, u); #endif /* SHADOWGRP */ - token = strtok(NULL, ","); } } diff --git a/src/login_nopam.c b/src/login_nopam.c index 9cba138b..56692e15 100644 --- a/src/login_nopam.c +++ b/src/login_nopam.c @@ -67,10 +67,6 @@ #define TABLE "/etc/login.access" #endif -/* Delimiters for fields and for lists of users, ttys or hosts. */ -static char fs[] = ":"; /* field separator */ -static char sep[] = ", \t"; /* list-element separator */ - static bool list_match (char *list, const char *item, bool (*match_fn) (const char *, const char *)); static bool user_match (const char *tok, const char *string); static bool from_match (const char *tok, const char *string); @@ -78,7 +74,8 @@ static bool string_match (const char *tok, const char *string); static const char *resolve_hostname (const char *string); /* login_access - match username/group and host/tty with access control file */ -int login_access (const char *user, const char *from) +int +login_access(const char *user, const char *from) { FILE *fp; char line[BUFSIZ]; @@ -98,7 +95,10 @@ int login_access (const char *user, const char *from) if (NULL != fp) { int lineno = 0; /* for diagnostics */ while ( !match - && (fgets (line, sizeof (line), fp) == line)) { + && (fgets (line, sizeof (line), fp) == line)) + { + char *p; + lineno++; if (stpsep(line, "\n") == NULL) { SYSLOG ((LOG_ERR, @@ -113,10 +113,11 @@ int login_access (const char *user, const char *from) if (line[0] == '\0') { /* skip blank lines */ continue; } - if ( ((perm = strtok (line, fs)) == NULL) - || ((users = strtok (NULL, fs)) == NULL) - || ((froms = strtok (NULL, fs)) == NULL) - || (strtok (NULL, fs) != NULL)) { + p = line; + perm = strsep(&p, ":"); + users = strsep(&p, ":"); + froms = strsep(&p, ":"); + if (froms == NULL || p != NULL) { SYSLOG ((LOG_ERR, "%s: line %d: bad field count", TABLE, lineno)); @@ -140,8 +141,11 @@ int login_access (const char *user, const char *from) } /* list_match - match an item against a list of tokens with exceptions */ -static bool list_match (char *list, const char *item, bool (*match_fn) (const char *, const char*)) +static bool +list_match(char *list, const char *item, bool (*match_fn)(const char *, const char*)) { + static const char sep[] = ", \t"; + char *tok; bool match = false; @@ -151,7 +155,7 @@ static bool list_match (char *list, const char *item, bool (*match_fn) (const ch * a match, look for an "EXCEPT" list and recurse to determine whether * the match is affected by any exceptions. */ - for (tok = strtok (list, sep); tok != NULL; tok = strtok (NULL, sep)) { + while (NULL != (tok = strsep(&list, sep))) { if (strcasecmp (tok, "EXCEPT") == 0) { /* EXCEPT: give up */ break; } @@ -163,7 +167,7 @@ static bool list_match (char *list, const char *item, bool (*match_fn) (const ch /* Process exceptions to matches. */ if (match) { - while ( ((tok = strtok (NULL, sep)) != NULL) + while ( (NULL != (tok = strsep(&list, sep))) && (strcasecmp (tok, "EXCEPT") != 0)) /* VOID */ ; if (tok == NULL || !list_match(NULL, item, match_fn)) { diff --git a/src/suauth.c b/src/suauth.c index ee035a02..936b3a2f 100644 --- a/src/suauth.c +++ b/src/suauth.c @@ -45,11 +45,9 @@ static int isgrp (const char *, const char *); static int lines = 0; -int check_su_auth (const char *actual_id, - const char *wanted_id, - bool su_to_root) +int +check_su_auth(const char *actual_id, const char *wanted_id, bool su_to_root) { - const char field[] = ":"; FILE *authfile_fd; char temp[1024]; char *to_users; @@ -91,10 +89,10 @@ int check_su_auth (const char *actual_id, if (*p == '#' || *p == '\0') continue; - if (!(to_users = strtok(p, field)) - || !(from_users = strtok (NULL, field)) - || !(action = strtok (NULL, field)) - || strtok (NULL, field)) { + to_users = strsep(&p, ":"); + from_users = strsep(&p, ":"); + action = strsep(&p, ":"); + if (action == NULL || p != NULL) { SYSLOG ((LOG_ERR, "%s, line %d. Bad number of fields.\n", SUAUTHFILE, lines)); @@ -138,15 +136,14 @@ int check_su_auth (const char *actual_id, return NOACTION; } -static int applies (const char *single, char *list) +static int +applies(const char *single, char *list) { - const char split[] = ", "; char *tok; int state = 0; - for (tok = strtok (list, split); tok != NULL; - tok = strtok (NULL, split)) { + while (NULL != (tok = strsep(&list, ", "))) { if (streq(tok, "ALL")) { if (state != 0) {