From: Todd C. Miller <Todd.Miller@sudo.ws>
Date: Mon, 5 Feb 2018 20:33:29 +0000 (-0700)
Subject: Refactor member freeing code into free_member().
X-Git-Tag: SUDO_1_8_23^2~148
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5cca4b6906f3da866cf46d696386a1c76435a657;p=sudo

Refactor member freeing code into free_member().
Refactor userspec freeing code into free_userspec().
---

diff --git a/plugins/sudoers/gram.c b/plugins/sudoers/gram.c
index 2109f4056..48cb692be 100644
--- a/plugins/sudoers/gram.c
+++ b/plugins/sudoers/gram.c
@@ -830,125 +830,120 @@ add_userspec(struct member *members, struct privilege *privs)
     debug_return_bool(true);
 }
 
+/*
+ * Free a member struct and its contents.
+ */
+void
+free_member(struct member *m)
+{
+    if (m->type == COMMAND) {
+	    struct sudo_command *c = (struct sudo_command *)m->name;
+	    free(c->cmnd);
+	    free(c->args);
+	    if (c->digest != NULL) {
+		free(c->digest->digest_str);
+		free(c->digest);
+	    }
+    }
+    free(m->name);
+    free(m);
+}
+
 /*
  * Free a tailq of members but not the struct member_list container itself.
  */
 void
 free_members(struct member_list *members)
 {
-    struct member *m, *next;
-    struct sudo_command *c;
+    struct member *m;
 
-    TAILQ_FOREACH_SAFE(m, members, entries, next) {
-	if (m->type == COMMAND) {
-		c = (struct sudo_command *) m->name;
-		free(c->cmnd);
-		free(c->args);
-	}
-	free(m->name);
-	free(m);
+    while ((m = TAILQ_FIRST(members)) != NULL) {
+	TAILQ_REMOVE(members, m, entries);
+	free_member(m);
     }
 }
 
-/*
- * Free up space used by data structures from a previous parser run and sets
- * the current sudoers file to path.
- */
-bool
-init_parser(const char *path, bool quiet)
+void
+free_userspec(struct userspec *us)
 {
-    struct member_list *binding;
-    struct defaults *d, *d_next;
-    struct userspec *us, *us_next;
-    bool ret = true;
-    debug_decl(init_parser, SUDOERS_DEBUG_PARSER)
+    struct privilege *priv, *next;
 
-    /* XXX - move into a free function */
-    TAILQ_FOREACH_SAFE(us, &userspecs, entries, us_next) {
-	struct member *m, *m_next;
-	struct privilege *priv, *priv_next;
-
-	TAILQ_FOREACH_SAFE(m, &us->users, entries, m_next) {
-	    free(m->name);
-	    free(m);
-	}
-	TAILQ_FOREACH_SAFE(priv, &us->privileges, entries, priv_next) {
-	    struct member_list *runasuserlist = NULL, *runasgrouplist = NULL;
-	    struct cmndspec *cs, *cs_next;
+    free_members(&us->users);
+    TAILQ_FOREACH_SAFE(priv, &us->privileges, entries, next) {
+	struct member_list *runasuserlist = NULL, *runasgrouplist = NULL;
+	struct cmndspec *cs, *cs_next;
 #ifdef HAVE_SELINUX
-	    char *role = NULL, *type = NULL;
+	char *role = NULL, *type = NULL;
 #endif /* HAVE_SELINUX */
 #ifdef HAVE_PRIV_SET
-	    char *privs = NULL, *limitprivs = NULL;
+	char *privs = NULL, *limitprivs = NULL;
 #endif /* HAVE_PRIV_SET */
 
-	    TAILQ_FOREACH_SAFE(m, &priv->hostlist, entries, m_next) {
-		free(m->name);
-		free(m);
-	    }
-	    TAILQ_FOREACH_SAFE(cs, &priv->cmndlist, entries, cs_next) {
+	free_members(&priv->hostlist);
+	TAILQ_FOREACH_SAFE(cs, &priv->cmndlist, entries, cs_next) {
 #ifdef HAVE_SELINUX
-		/* Only free the first instance of a role/type. */
-		if (cs->role != role) {
-		    role = cs->role;
-		    free(cs->role);
-		}
-		if (cs->type != type) {
-		    type = cs->type;
-		    free(cs->type);
-		}
+	    /* Only free the first instance of a role/type. */
+	    if (cs->role != role) {
+		role = cs->role;
+		free(cs->role);
+	    }
+	    if (cs->type != type) {
+		type = cs->type;
+		free(cs->type);
+	    }
 #endif /* HAVE_SELINUX */
 #ifdef HAVE_PRIV_SET
-		/* Only free the first instance of privs/limitprivs. */
-		if (cs->privs != privs) {
-		    privs = cs->privs;
-		    free(cs->privs);
-		}
-		if (cs->limitprivs != limitprivs) {
-		    limitprivs = cs->limitprivs;
-		    free(cs->limitprivs);
-		}
+	    /* Only free the first instance of privs/limitprivs. */
+	    if (cs->privs != privs) {
+		privs = cs->privs;
+		free(cs->privs);
+	    }
+	    if (cs->limitprivs != limitprivs) {
+		limitprivs = cs->limitprivs;
+		free(cs->limitprivs);
+	    }
 #endif /* HAVE_PRIV_SET */
-		/* Only free the first instance of runas user/group lists. */
-		if (cs->runasuserlist && cs->runasuserlist != runasuserlist) {
-		    runasuserlist = cs->runasuserlist;
-		    TAILQ_FOREACH_SAFE(m, runasuserlist, entries, m_next) {
-			free(m->name);
-			free(m);
-		    }
-		    free(runasuserlist);
-		}
-		if (cs->runasgrouplist && cs->runasgrouplist != runasgrouplist) {
-		    runasgrouplist = cs->runasgrouplist;
-		    TAILQ_FOREACH_SAFE(m, runasgrouplist, entries, m_next) {
-			free(m->name);
-			free(m);
-		    }
-		    free(runasgrouplist);
-		}
-		if (cs->cmnd->type == COMMAND) {
-			struct sudo_command *c =
-			    (struct sudo_command *) cs->cmnd->name;
-			free(c->cmnd);
-			free(c->args);
-			if (c->digest != NULL) {
-			    free(c->digest->digest_str);
-			    free(c->digest);
-			}
-		}
-		free(cs->cmnd->name);
-		free(cs->cmnd);
-		free(cs);
+	    /* Only free the first instance of runas user/group lists. */
+	    if (cs->runasuserlist && cs->runasuserlist != runasuserlist) {
+		runasuserlist = cs->runasuserlist;
+		free_members(runasuserlist);
+		free(runasuserlist);
+	    }
+	    if (cs->runasgrouplist && cs->runasgrouplist != runasgrouplist) {
+		runasgrouplist = cs->runasgrouplist;
+		free_members(runasgrouplist);
+		free(runasgrouplist);
 	    }
-	    free(priv);
+	    free_member(cs->cmnd);
+	    free(cs);
 	}
-	rcstr_delref(us->file);
-	free(us);
+	free(priv);
+    }
+    rcstr_delref(us->file);
+    free(us);
+}
+
+/*
+ * Free up space used by data structures from a previous parser run and sets
+ * the current sudoers file to path.
+ */
+bool
+init_parser(const char *path, bool quiet)
+{
+    struct member_list *binding;
+    struct defaults *d;
+    struct userspec *us;
+    bool ret = true;
+    void *next;
+    debug_decl(init_parser, SUDOERS_DEBUG_PARSER)
+
+    TAILQ_FOREACH_SAFE(us, &userspecs, entries, next) {
+	free_userspec(us);
     }
     TAILQ_INIT(&userspecs);
 
     binding = NULL;
-    TAILQ_FOREACH_SAFE(d, &defaults, entries, d_next) {
+    TAILQ_FOREACH_SAFE(d, &defaults, entries, next) {
 	if (d->binding != binding) {
 	    binding = d->binding;
 	    free_members(d->binding);
@@ -1005,7 +1000,7 @@ init_options(struct command_options *opts)
     opts->limitprivs = NULL;
 #endif
 }
-#line 956 "gram.c"
+#line 951 "gram.c"
 /* allocate initial stack or double stack size, up to YYMAXDEPTH */
 #if defined(__cplusplus) || defined(__STDC__)
 static int yygrowstack(void)
@@ -2129,7 +2124,7 @@ case 116:
 			    }
 			}
 break;
-#line 2080 "gram.c"
+#line 2075 "gram.c"
     }
     yyssp -= yym;
     yystate = *yyssp;
diff --git a/plugins/sudoers/gram.y b/plugins/sudoers/gram.y
index a428eb000..ed20fac00 100644
--- a/plugins/sudoers/gram.y
+++ b/plugins/sudoers/gram.y
@@ -1058,124 +1058,119 @@ add_userspec(struct member *members, struct privilege *privs)
 }
 
 /*
- * Free a tailq of members but not the struct member_list container itself.
+ * Free a member struct and its contents.
  */
 void
-free_members(struct member_list *members)
+free_member(struct member *m)
 {
-    struct member *m, *next;
-    struct sudo_command *c;
-
-    TAILQ_FOREACH_SAFE(m, members, entries, next) {
-	if (m->type == COMMAND) {
-		c = (struct sudo_command *) m->name;
-		free(c->cmnd);
-		free(c->args);
-	}
-	free(m->name);
-	free(m);
+    if (m->type == COMMAND) {
+	    struct sudo_command *c = (struct sudo_command *)m->name;
+	    free(c->cmnd);
+	    free(c->args);
+	    if (c->digest != NULL) {
+		free(c->digest->digest_str);
+		free(c->digest);
+	    }
     }
+    free(m->name);
+    free(m);
 }
 
 /*
- * Free up space used by data structures from a previous parser run and sets
- * the current sudoers file to path.
+ * Free a tailq of members but not the struct member_list container itself.
  */
-bool
-init_parser(const char *path, bool quiet)
+void
+free_members(struct member_list *members)
 {
-    struct member_list *binding;
-    struct defaults *d, *d_next;
-    struct userspec *us, *us_next;
-    bool ret = true;
-    debug_decl(init_parser, SUDOERS_DEBUG_PARSER)
+    struct member *m;
+
+    while ((m = TAILQ_FIRST(members)) != NULL) {
+	TAILQ_REMOVE(members, m, entries);
+	free_member(m);
+    }
+}
 
-    /* XXX - move into a free function */
-    TAILQ_FOREACH_SAFE(us, &userspecs, entries, us_next) {
-	struct member *m, *m_next;
-	struct privilege *priv, *priv_next;
+void
+free_userspec(struct userspec *us)
+{
+    struct privilege *priv, *next;
 
-	TAILQ_FOREACH_SAFE(m, &us->users, entries, m_next) {
-	    free(m->name);
-	    free(m);
-	}
-	TAILQ_FOREACH_SAFE(priv, &us->privileges, entries, priv_next) {
-	    struct member_list *runasuserlist = NULL, *runasgrouplist = NULL;
-	    struct cmndspec *cs, *cs_next;
+    free_members(&us->users);
+    TAILQ_FOREACH_SAFE(priv, &us->privileges, entries, next) {
+	struct member_list *runasuserlist = NULL, *runasgrouplist = NULL;
+	struct cmndspec *cs, *cs_next;
 #ifdef HAVE_SELINUX
-	    char *role = NULL, *type = NULL;
+	char *role = NULL, *type = NULL;
 #endif /* HAVE_SELINUX */
 #ifdef HAVE_PRIV_SET
-	    char *privs = NULL, *limitprivs = NULL;
+	char *privs = NULL, *limitprivs = NULL;
 #endif /* HAVE_PRIV_SET */
 
-	    TAILQ_FOREACH_SAFE(m, &priv->hostlist, entries, m_next) {
-		free(m->name);
-		free(m);
-	    }
-	    TAILQ_FOREACH_SAFE(cs, &priv->cmndlist, entries, cs_next) {
+	free_members(&priv->hostlist);
+	TAILQ_FOREACH_SAFE(cs, &priv->cmndlist, entries, cs_next) {
 #ifdef HAVE_SELINUX
-		/* Only free the first instance of a role/type. */
-		if (cs->role != role) {
-		    role = cs->role;
-		    free(cs->role);
-		}
-		if (cs->type != type) {
-		    type = cs->type;
-		    free(cs->type);
-		}
+	    /* Only free the first instance of a role/type. */
+	    if (cs->role != role) {
+		role = cs->role;
+		free(cs->role);
+	    }
+	    if (cs->type != type) {
+		type = cs->type;
+		free(cs->type);
+	    }
 #endif /* HAVE_SELINUX */
 #ifdef HAVE_PRIV_SET
-		/* Only free the first instance of privs/limitprivs. */
-		if (cs->privs != privs) {
-		    privs = cs->privs;
-		    free(cs->privs);
-		}
-		if (cs->limitprivs != limitprivs) {
-		    limitprivs = cs->limitprivs;
-		    free(cs->limitprivs);
-		}
+	    /* Only free the first instance of privs/limitprivs. */
+	    if (cs->privs != privs) {
+		privs = cs->privs;
+		free(cs->privs);
+	    }
+	    if (cs->limitprivs != limitprivs) {
+		limitprivs = cs->limitprivs;
+		free(cs->limitprivs);
+	    }
 #endif /* HAVE_PRIV_SET */
-		/* Only free the first instance of runas user/group lists. */
-		if (cs->runasuserlist && cs->runasuserlist != runasuserlist) {
-		    runasuserlist = cs->runasuserlist;
-		    TAILQ_FOREACH_SAFE(m, runasuserlist, entries, m_next) {
-			free(m->name);
-			free(m);
-		    }
-		    free(runasuserlist);
-		}
-		if (cs->runasgrouplist && cs->runasgrouplist != runasgrouplist) {
-		    runasgrouplist = cs->runasgrouplist;
-		    TAILQ_FOREACH_SAFE(m, runasgrouplist, entries, m_next) {
-			free(m->name);
-			free(m);
-		    }
-		    free(runasgrouplist);
-		}
-		if (cs->cmnd->type == COMMAND) {
-			struct sudo_command *c =
-			    (struct sudo_command *) cs->cmnd->name;
-			free(c->cmnd);
-			free(c->args);
-			if (c->digest != NULL) {
-			    free(c->digest->digest_str);
-			    free(c->digest);
-			}
-		}
-		free(cs->cmnd->name);
-		free(cs->cmnd);
-		free(cs);
+	    /* Only free the first instance of runas user/group lists. */
+	    if (cs->runasuserlist && cs->runasuserlist != runasuserlist) {
+		runasuserlist = cs->runasuserlist;
+		free_members(runasuserlist);
+		free(runasuserlist);
+	    }
+	    if (cs->runasgrouplist && cs->runasgrouplist != runasgrouplist) {
+		runasgrouplist = cs->runasgrouplist;
+		free_members(runasgrouplist);
+		free(runasgrouplist);
 	    }
-	    free(priv);
+	    free_member(cs->cmnd);
+	    free(cs);
 	}
-	rcstr_delref(us->file);
-	free(us);
+	free(priv);
+    }
+    rcstr_delref(us->file);
+    free(us);
+}
+
+/*
+ * Free up space used by data structures from a previous parser run and sets
+ * the current sudoers file to path.
+ */
+bool
+init_parser(const char *path, bool quiet)
+{
+    struct member_list *binding;
+    struct defaults *d;
+    struct userspec *us;
+    bool ret = true;
+    void *next;
+    debug_decl(init_parser, SUDOERS_DEBUG_PARSER)
+
+    TAILQ_FOREACH_SAFE(us, &userspecs, entries, next) {
+	free_userspec(us);
     }
     TAILQ_INIT(&userspecs);
 
     binding = NULL;
-    TAILQ_FOREACH_SAFE(d, &defaults, entries, d_next) {
+    TAILQ_FOREACH_SAFE(d, &defaults, entries, next) {
 	if (d->binding != binding) {
 	    binding = d->binding;
 	    free_members(d->binding);
diff --git a/plugins/sudoers/parse.h b/plugins/sudoers/parse.h
index 5638d88f0..a2701824b 100644
--- a/plugins/sudoers/parse.h
+++ b/plugins/sudoers/parse.h
@@ -255,7 +255,9 @@ bool init_aliases(void);
 
 /* gram.c */
 bool init_parser(const char *path, bool quiet);
+void free_member(struct member *m);
 void free_members(struct member_list *members);
+void free_userspec(struct userspec *us);
 
 /* match_addr.c */
 bool addr_matches(char *n);