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 <alx@kernel.org>

diff --git a/lib/addgrps.c b/lib/addgrps.c
index b2e17a9ddf09144b43be2e83457b35284d9cd802..97c47e077f3b69a6e6b187b1a178e0513f65f4fa 100644 (file)
--- a/lib/addgrps.c
+++ b/lib/addgrps.c
@@ -14,9 +14,10 @@
 #include "prototypes.h"
 #include "defines.h"
 
-#include <stdio.h>
-#include <grp.h>
 #include <errno.h>
+#include <grp.h>
+#include <stdio.h>
+#include <string.h>
 
 #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 b9f088d2d3517a829d9819e3c7d90d2033e41375..283893311e07c87947614d0da5d49c63d9fa71c0 100644 (file)
--- 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 52712675f5d45309782d0e3a35bfd9f05fc3e048..6394dbd945d647dace4e8026ed0e20a445ee1c1d 100644 (file)
--- a/lib/motd.c
+++ b/lib/motd.c
@@ -12,6 +12,7 @@
 #ident "$Id$"
 
 #include <stdio.h>
+#include <string.h>
 
 #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 263dc93ae42f7417051012b8bb674ea56a785f6e..9f0eb2e50f009085a91dcdfc28196dec1762fd79 100644 (file)
--- a/src/groupadd.c
+++ b/src/groupadd.c
@@ -16,6 +16,7 @@
 #include <getopt.h>
 #include <grp.h>
 #include <stdio.h>
+#include <string.h>
 #include <sys/types.h>
 #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 ee1b4a9d3e90352377d57358f0da48fbea06e8fa..7342707d06159191bc9ea31431911f9dc1ff613f 100644 (file)
--- a/src/groupmod.c
+++ b/src/groupmod.c
@@ -17,6 +17,7 @@
 #include <grp.h>
 #include <stdint.h>
 #include <stdio.h>
+#include <string.h>
 #include <strings.h>
 #include <sys/types.h>
 #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 9cba138b69fa767471b533d96dbf267a91bbfa59..56692e15d6f9edd804f5f13860e4801bbe1644e1 100644 (file)
--- 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 ee035a02867411466b3f69df508312dd3dd55607..936b3a2f2e0d1d5f1970a3409004f62e59c5df89 100644 (file)
--- 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) {