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 <[email protected]>

lib/addgrps.c

@@ -14,29 +14,30 @@
 #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"
 #include "shadowlog.h"
 
 #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;
 		}
 

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;
 	}

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) {

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, ",");
 		}
 	}
 

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, ",");
 		}
 	}
 

src/login_nopam.c

@@ -67,18 +67,15 @@
 #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);
 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)) {

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) {