Quiet some llvm check false positives. The common idiom of using

TAILQ_FIRST, TAILQ_REMOVE and free in a loop to free each entry in
a TAILQ confuses llvm.  Use TAILQ_FOREACH_SAFE instead (which is
probably faster anyway).
This commit is contained in:
Todd C. Miller
2013-10-22 14:58:00 -06:00
parent 65c6f34aa4
commit e8ce021e7d
3 changed files with 89 additions and 95 deletions

View File

@@ -788,24 +788,26 @@ add_defaults(int type, struct member *bmem, struct defaults *defs)
struct member_list *binding; struct member_list *binding;
debug_decl(add_defaults, SUDO_DEBUG_PARSER) debug_decl(add_defaults, SUDO_DEBUG_PARSER)
/* if (defs != NULL) {
* We use a single binding for each entry in defs. /*
*/ * We use a single binding for each entry in defs.
binding = emalloc(sizeof(*binding)); */
if (bmem != NULL) binding = emalloc(sizeof(*binding));
HLTQ_TO_TAILQ(binding, bmem, entries); if (bmem != NULL)
else HLTQ_TO_TAILQ(binding, bmem, entries);
TAILQ_INIT(binding); else
TAILQ_INIT(binding);
/* /*
* Set type and binding (who it applies to) for new entries. * Set type and binding (who it applies to) for new entries.
* Then add to the global defaults list. * Then add to the global defaults list.
*/ */
HLTQ_FOREACH(d, defs, entries) { HLTQ_FOREACH(d, defs, entries) {
d->type = type; d->type = type;
d->binding = binding; d->binding = binding;
}
TAILQ_CONCAT_HLTQ(&defaults, defs, entries);
} }
TAILQ_CONCAT_HLTQ(&defaults, defs, entries);
debug_return; debug_return;
} }
@@ -836,23 +838,21 @@ void
init_parser(const char *path, bool quiet) init_parser(const char *path, bool quiet)
{ {
struct member_list *binding; struct member_list *binding;
struct member *m; struct defaults *d, *d_next;
struct defaults *d; struct userspec *us, *us_next;
struct userspec *us;
struct privilege *priv;
struct cmndspec *cs;
struct sudo_command *c;
debug_decl(init_parser, SUDO_DEBUG_PARSER) debug_decl(init_parser, SUDO_DEBUG_PARSER)
while ((us = TAILQ_FIRST(&userspecs)) != NULL) { TAILQ_FOREACH_SAFE(us, &userspecs, entries, us_next) {
TAILQ_REMOVE(&userspecs, us, entries); struct member *m, *m_next;
while ((m = TAILQ_FIRST(&us->users)) != NULL) { struct privilege *priv, *priv_next;
TAILQ_REMOVE(&us->users, m, entries);
TAILQ_FOREACH_SAFE(m, &us->users, entries, m_next) {
efree(m->name); efree(m->name);
efree(m); efree(m);
} }
while ((priv = TAILQ_FIRST(&us->privileges)) != NULL) { TAILQ_FOREACH_SAFE(priv, &us->privileges, entries, priv_next) {
struct member_list *runasuserlist = NULL, *runasgrouplist = NULL; struct member_list *runasuserlist = NULL, *runasgrouplist = NULL;
struct cmndspec *cs, *cs_next;
#ifdef HAVE_SELINUX #ifdef HAVE_SELINUX
char *role = NULL, *type = NULL; char *role = NULL, *type = NULL;
#endif /* HAVE_SELINUX */ #endif /* HAVE_SELINUX */
@@ -860,14 +860,11 @@ init_parser(const char *path, bool quiet)
char *privs = NULL, *limitprivs = NULL; char *privs = NULL, *limitprivs = NULL;
#endif /* HAVE_PRIV_SET */ #endif /* HAVE_PRIV_SET */
TAILQ_REMOVE(&us->privileges, priv, entries); TAILQ_FOREACH_SAFE(m, &priv->hostlist, entries, m_next) {
while ((m = TAILQ_FIRST(&priv->hostlist)) != NULL) {
TAILQ_REMOVE(&priv->hostlist, m, entries);
efree(m->name); efree(m->name);
efree(m); efree(m);
} }
while ((cs = TAILQ_FIRST(&priv->cmndlist)) != NULL) { TAILQ_FOREACH_SAFE(cs, &priv->cmndlist, entries, cs_next) {
TAILQ_REMOVE(&priv->cmndlist, cs, entries);
#ifdef HAVE_SELINUX #ifdef HAVE_SELINUX
/* Only free the first instance of a role/type. */ /* Only free the first instance of a role/type. */
if (cs->role != role) { if (cs->role != role) {
@@ -893,8 +890,7 @@ init_parser(const char *path, bool quiet)
/* Only free the first instance of runas user/group lists. */ /* Only free the first instance of runas user/group lists. */
if (cs->runasuserlist && cs->runasuserlist != runasuserlist) { if (cs->runasuserlist && cs->runasuserlist != runasuserlist) {
runasuserlist = cs->runasuserlist; runasuserlist = cs->runasuserlist;
while ((m = TAILQ_FIRST(runasuserlist)) != NULL) { TAILQ_FOREACH_SAFE(m, runasuserlist, entries, m_next) {
TAILQ_REMOVE(runasuserlist, m, entries);
efree(m->name); efree(m->name);
efree(m); efree(m);
} }
@@ -902,15 +898,15 @@ init_parser(const char *path, bool quiet)
} }
if (cs->runasgrouplist && cs->runasgrouplist != runasgrouplist) { if (cs->runasgrouplist && cs->runasgrouplist != runasgrouplist) {
runasgrouplist = cs->runasgrouplist; runasgrouplist = cs->runasgrouplist;
while ((m = TAILQ_FIRST(runasgrouplist)) != NULL) { TAILQ_FOREACH_SAFE(m, runasgrouplist, entries, m_next) {
TAILQ_REMOVE(runasgrouplist, m, entries);
efree(m->name); efree(m->name);
efree(m); efree(m);
} }
efree(runasgrouplist); efree(runasgrouplist);
} }
if (cs->cmnd->type == COMMAND) { if (cs->cmnd->type == COMMAND) {
c = (struct sudo_command *) cs->cmnd->name; struct sudo_command *c =
(struct sudo_command *) cs->cmnd->name;
efree(c->cmnd); efree(c->cmnd);
efree(c->args); efree(c->args);
} }
@@ -925,14 +921,15 @@ init_parser(const char *path, bool quiet)
TAILQ_INIT(&userspecs); TAILQ_INIT(&userspecs);
binding = NULL; binding = NULL;
while ((d = TAILQ_FIRST(&defaults)) != NULL) { TAILQ_FOREACH_SAFE(d, &defaults, entries, d_next) {
TAILQ_REMOVE(&defaults, d, entries);
if (d->binding != binding) { if (d->binding != binding) {
struct member *m, *m_next;
binding = d->binding; binding = d->binding;
while ((m = TAILQ_FIRST(d->binding)) != NULL) { TAILQ_FOREACH_SAFE(m, d->binding, entries, m_next) {
TAILQ_REMOVE(d->binding, m, entries);
if (m->type == COMMAND) { if (m->type == COMMAND) {
c = (struct sudo_command *) m->name; struct sudo_command *c =
(struct sudo_command *) m->name;
efree(c->cmnd); efree(c->cmnd);
efree(c->args); efree(c->args);
} }
@@ -961,7 +958,7 @@ init_parser(const char *path, bool quiet)
debug_return; debug_return;
} }
#line 912 "gram.c" #line 909 "gram.c"
/* allocate initial stack or double stack size, up to YYMAXDEPTH */ /* allocate initial stack or double stack size, up to YYMAXDEPTH */
#if defined(__cplusplus) || defined(__STDC__) #if defined(__cplusplus) || defined(__STDC__)
static int yygrowstack(void) static int yygrowstack(void)
@@ -1847,7 +1844,7 @@ case 111:
yyval.member = new_member(yyvsp[0].string, WORD); yyval.member = new_member(yyvsp[0].string, WORD);
} }
break; break;
#line 1798 "gram.c" #line 1795 "gram.c"
} }
yyssp -= yym; yyssp -= yym;
yystate = *yyssp; yystate = *yyssp;

View File

@@ -773,24 +773,26 @@ add_defaults(int type, struct member *bmem, struct defaults *defs)
struct member_list *binding; struct member_list *binding;
debug_decl(add_defaults, SUDO_DEBUG_PARSER) debug_decl(add_defaults, SUDO_DEBUG_PARSER)
/* if (defs != NULL) {
* We use a single binding for each entry in defs. /*
*/ * We use a single binding for each entry in defs.
binding = emalloc(sizeof(*binding)); */
if (bmem != NULL) binding = emalloc(sizeof(*binding));
HLTQ_TO_TAILQ(binding, bmem, entries); if (bmem != NULL)
else HLTQ_TO_TAILQ(binding, bmem, entries);
TAILQ_INIT(binding); else
TAILQ_INIT(binding);
/* /*
* Set type and binding (who it applies to) for new entries. * Set type and binding (who it applies to) for new entries.
* Then add to the global defaults list. * Then add to the global defaults list.
*/ */
HLTQ_FOREACH(d, defs, entries) { HLTQ_FOREACH(d, defs, entries) {
d->type = type; d->type = type;
d->binding = binding; d->binding = binding;
}
TAILQ_CONCAT_HLTQ(&defaults, defs, entries);
} }
TAILQ_CONCAT_HLTQ(&defaults, defs, entries);
debug_return; debug_return;
} }
@@ -821,23 +823,21 @@ void
init_parser(const char *path, bool quiet) init_parser(const char *path, bool quiet)
{ {
struct member_list *binding; struct member_list *binding;
struct member *m; struct defaults *d, *d_next;
struct defaults *d; struct userspec *us, *us_next;
struct userspec *us;
struct privilege *priv;
struct cmndspec *cs;
struct sudo_command *c;
debug_decl(init_parser, SUDO_DEBUG_PARSER) debug_decl(init_parser, SUDO_DEBUG_PARSER)
while ((us = TAILQ_FIRST(&userspecs)) != NULL) { TAILQ_FOREACH_SAFE(us, &userspecs, entries, us_next) {
TAILQ_REMOVE(&userspecs, us, entries); struct member *m, *m_next;
while ((m = TAILQ_FIRST(&us->users)) != NULL) { struct privilege *priv, *priv_next;
TAILQ_REMOVE(&us->users, m, entries);
TAILQ_FOREACH_SAFE(m, &us->users, entries, m_next) {
efree(m->name); efree(m->name);
efree(m); efree(m);
} }
while ((priv = TAILQ_FIRST(&us->privileges)) != NULL) { TAILQ_FOREACH_SAFE(priv, &us->privileges, entries, priv_next) {
struct member_list *runasuserlist = NULL, *runasgrouplist = NULL; struct member_list *runasuserlist = NULL, *runasgrouplist = NULL;
struct cmndspec *cs, *cs_next;
#ifdef HAVE_SELINUX #ifdef HAVE_SELINUX
char *role = NULL, *type = NULL; char *role = NULL, *type = NULL;
#endif /* HAVE_SELINUX */ #endif /* HAVE_SELINUX */
@@ -845,14 +845,11 @@ init_parser(const char *path, bool quiet)
char *privs = NULL, *limitprivs = NULL; char *privs = NULL, *limitprivs = NULL;
#endif /* HAVE_PRIV_SET */ #endif /* HAVE_PRIV_SET */
TAILQ_REMOVE(&us->privileges, priv, entries); TAILQ_FOREACH_SAFE(m, &priv->hostlist, entries, m_next) {
while ((m = TAILQ_FIRST(&priv->hostlist)) != NULL) {
TAILQ_REMOVE(&priv->hostlist, m, entries);
efree(m->name); efree(m->name);
efree(m); efree(m);
} }
while ((cs = TAILQ_FIRST(&priv->cmndlist)) != NULL) { TAILQ_FOREACH_SAFE(cs, &priv->cmndlist, entries, cs_next) {
TAILQ_REMOVE(&priv->cmndlist, cs, entries);
#ifdef HAVE_SELINUX #ifdef HAVE_SELINUX
/* Only free the first instance of a role/type. */ /* Only free the first instance of a role/type. */
if (cs->role != role) { if (cs->role != role) {
@@ -878,8 +875,7 @@ init_parser(const char *path, bool quiet)
/* Only free the first instance of runas user/group lists. */ /* Only free the first instance of runas user/group lists. */
if (cs->runasuserlist && cs->runasuserlist != runasuserlist) { if (cs->runasuserlist && cs->runasuserlist != runasuserlist) {
runasuserlist = cs->runasuserlist; runasuserlist = cs->runasuserlist;
while ((m = TAILQ_FIRST(runasuserlist)) != NULL) { TAILQ_FOREACH_SAFE(m, runasuserlist, entries, m_next) {
TAILQ_REMOVE(runasuserlist, m, entries);
efree(m->name); efree(m->name);
efree(m); efree(m);
} }
@@ -887,15 +883,15 @@ init_parser(const char *path, bool quiet)
} }
if (cs->runasgrouplist && cs->runasgrouplist != runasgrouplist) { if (cs->runasgrouplist && cs->runasgrouplist != runasgrouplist) {
runasgrouplist = cs->runasgrouplist; runasgrouplist = cs->runasgrouplist;
while ((m = TAILQ_FIRST(runasgrouplist)) != NULL) { TAILQ_FOREACH_SAFE(m, runasgrouplist, entries, m_next) {
TAILQ_REMOVE(runasgrouplist, m, entries);
efree(m->name); efree(m->name);
efree(m); efree(m);
} }
efree(runasgrouplist); efree(runasgrouplist);
} }
if (cs->cmnd->type == COMMAND) { if (cs->cmnd->type == COMMAND) {
c = (struct sudo_command *) cs->cmnd->name; struct sudo_command *c =
(struct sudo_command *) cs->cmnd->name;
efree(c->cmnd); efree(c->cmnd);
efree(c->args); efree(c->args);
} }
@@ -910,14 +906,15 @@ init_parser(const char *path, bool quiet)
TAILQ_INIT(&userspecs); TAILQ_INIT(&userspecs);
binding = NULL; binding = NULL;
while ((d = TAILQ_FIRST(&defaults)) != NULL) { TAILQ_FOREACH_SAFE(d, &defaults, entries, d_next) {
TAILQ_REMOVE(&defaults, d, entries);
if (d->binding != binding) { if (d->binding != binding) {
struct member *m, *m_next;
binding = d->binding; binding = d->binding;
while ((m = TAILQ_FIRST(d->binding)) != NULL) { TAILQ_FOREACH_SAFE(m, d->binding, entries, m_next) {
TAILQ_REMOVE(d->binding, m, entries);
if (m->type == COMMAND) { if (m->type == COMMAND) {
c = (struct sudo_command *) m->name; struct sudo_command *c =
(struct sudo_command *) m->name;
efree(c->cmnd); efree(c->cmnd);
efree(c->args); efree(c->args);
} }

View File

@@ -337,13 +337,14 @@ exec_event_setup(int backchannel, struct exec_closure *ec)
int int
sudo_execute(struct command_details *details, struct command_status *cstat) sudo_execute(struct command_details *details, struct command_status *cstat)
{ {
int sv[2]; struct sigforward *sigfwd, *sigfwd_next;
const char *utmp_user = NULL; const char *utmp_user = NULL;
struct sudo_event_base *evbase; struct sudo_event_base *evbase;
struct exec_closure ec; struct exec_closure ec;
bool log_io = false; bool log_io = false;
sigaction_t sa; sigaction_t sa;
pid_t child; pid_t child;
int sv[2];
debug_decl(sudo_execute, SUDO_DEBUG_EXEC) debug_decl(sudo_execute, SUDO_DEBUG_EXEC)
dispatch_pending_signals(cstat); dispatch_pending_signals(cstat);
@@ -493,15 +494,14 @@ sudo_execute(struct command_details *details, struct command_status *cstat)
#endif #endif
/* Free things up. */ /* Free things up. */
while (!TAILQ_EMPTY(&sigfwd_list)) {
struct sigforward *sigfwd = TAILQ_FIRST(&sigfwd_list);
TAILQ_REMOVE(&sigfwd_list, sigfwd, entries);
efree(sigfwd);
}
sudo_ev_free(sigfwd_event); sudo_ev_free(sigfwd_event);
sudo_ev_free(signal_event); sudo_ev_free(signal_event);
sudo_ev_free(backchannel_event); sudo_ev_free(backchannel_event);
sudo_ev_base_free(evbase); sudo_ev_base_free(evbase);
TAILQ_FOREACH_SAFE(sigfwd, &sigfwd_list, entries, sigfwd_next) {
efree(sigfwd);
}
TAILQ_INIT(&sigfwd_list);
done: done:
debug_return_int(cstat->type == CMD_ERRNO ? -1 : 0); debug_return_int(cstat->type == CMD_ERRNO ? -1 : 0);
} }
@@ -800,14 +800,14 @@ forward_signals(int sock, int what, void *v)
efree(sigfwd); efree(sigfwd);
if (nsent != sizeof(cstat)) { if (nsent != sizeof(cstat)) {
if (errno == EPIPE) { if (errno == EPIPE) {
struct sigforward *sigfwd_next;
sudo_debug_printf(SUDO_DEBUG_ERROR, sudo_debug_printf(SUDO_DEBUG_ERROR,
"broken pipe writing to child over backchannel"); "broken pipe writing to child over backchannel");
/* Other end of socket gone, empty out sigfwd_list. */ /* Other end of socket gone, empty out sigfwd_list. */
while (!TAILQ_EMPTY(&sigfwd_list)) { TAILQ_FOREACH_SAFE(sigfwd, &sigfwd_list, entries, sigfwd_next) {
sigfwd = TAILQ_FIRST(&sigfwd_list);
TAILQ_REMOVE(&sigfwd_list, sigfwd, entries);
efree(sigfwd); efree(sigfwd);
} }
TAILQ_INIT(&sigfwd_list);
/* XXX - child (monitor) is dead, we should exit too? */ /* XXX - child (monitor) is dead, we should exit too? */
} }
break; break;