Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: PATCH: removal of dead code identified by scan-build



I've had a similar patch lying around for a while (zw/29270) which had
a few remaining hunks after rebasing it on top of this. Two of them
looked a little scary, but these remaining two are pretty
straightforward dead assignment removals. The third remaining one I'll
send in a separate patch along with a real(ish) issue in the same
function.

(I've pasted this in gmail, so this is illustrative, I'll push it
after Oliver's is in.).

diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index 58daec01af..43018c2e51 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -4679,7 +4679,7 @@ cfp_opt_pats(char **pats, char *matcher)
             q = dupstring(q);
             t = q + strlen(q) - 1;
             if (*t == ')') {
-                for (s = t--; t > q; t--)
+                while (--t > q)
                     if (*t == ')' || *t == '|' || *t == '~' || *t == '(')
                         break;
                 if (t != q && *t == '(')
diff --git a/Src/module.c b/Src/module.c
index 0b5cd56493..5652a123ee 100644
--- a/Src/module.c
+++ b/Src/module.c
@@ -426,14 +426,13 @@ static int
 add_autobin(const char *module, const char *bnam, int flags)
 {
     Builtin bn;
-    int ret;

     bn = zshcalloc(sizeof(*bn));
     bn->node.nam = ztrdup(bnam);
     bn->optstr = ztrdup(module);
     if (flags & FEAT_AUTOALL)
         bn->node.flags |= BINF_AUTOALL;
-    if ((ret = addbuiltin(bn))) {
+    if (addbuiltin(bn)) {
         builtintab->freenode(&bn->node);
         if (!(flags & FEAT_IGNORE))
             return 1;

On Fri, May 22, 2026 at 1:39 AM Oliver Kiddle <opk@xxxxxxx> wrote:
>
> Looking over the results of scan-build, quite a few of the identified
> items are dead code. These are not as difficult to verify as some of the
> other issues so the following corrects many of those. There's a good few
> cases that I left some alone: e.g. where I suspect that removing an
> initialisation might trigger a compiler warning or make it more likely
> that a future changes will use a variable uninitialsed.
>
> The dead code in execsubst() was saving and restoring a LinkList that
> was a passed by value pointer. Perhaps the intention was that the whole
> list be saved and restored?
>
> Oliver
>
>
> diff --git a/Src/Modules/socket.c b/Src/Modules/socket.c
> index 2c8a48c28..3ecab42c9 100644
> --- a/Src/Modules/socket.c
> +++ b/Src/Modules/socket.c
> @@ -56,7 +56,7 @@
>  static int
>  bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
>  {
> -    int err=1, verbose=0, test=0, targetfd=0;
> +    int verbose=0, test=0, targetfd=0;
>      ZSOCKLEN_T len;
>      struct sockaddr_un soun = { 0 };
>      int sfd;
> @@ -236,8 +236,7 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
>             return 1;
>         }
>
> -       int err = shutdown(atoi(args[0]), SHUT_WR);
> -       if (err)
> +       if (shutdown(atoi(args[0]), SHUT_WR))
>             zwarn("shutdown failed: %e", errno);
>      }
>      else
> @@ -262,7 +261,7 @@ bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
>         soun.sun_family = AF_UNIX;
>         strncpy(soun.sun_path, args[0], sizeof(soun.sun_path)-1);
>
> -       if ((err = connect(sfd, (struct sockaddr *)&soun, sizeof(struct sockaddr_un)))) {
> +       if (connect(sfd, (struct sockaddr *)&soun, sizeof(struct sockaddr_un))) {
>             zwarnnam(nam, "connection failed: %e", errno);
>             close(sfd);
>             return 1;
> diff --git a/Src/Modules/zftp.c b/Src/Modules/zftp.c
> index ecabf3430..29170ad11 100644
> --- a/Src/Modules/zftp.c
> +++ b/Src/Modules/zftp.c
> @@ -2034,7 +2034,7 @@ zfgetinfo(char *prompt, int noecho)
>      }
>
>      if (fgets(instr, 256, stdin) == NULL)
> -       instr[len = 0] = '\0';
> +       instr[0] = '\0';
>      else if (instr[len = strlen(instr)-1] == '\n')
>         instr[len] = '\0';
>
> @@ -2559,7 +2559,7 @@ zftp_getput(char *name, char **args, int flags)
>      for (; *args; args++) {
>         char *ln, *rest = NULL;
>         off_t startat = 0;
> -       if (progress && (shfunc = getshfunc("zftp_progress"))) {
> +       if (progress && getshfunc("zftp_progress")) {
>             off_t sz = -1;
>             /*
>              * This calls the SIZE command to get the size for remote
> diff --git a/Src/Modules/zpty.c b/Src/Modules/zpty.c
> index d87e32744..de450be7a 100644
> --- a/Src/Modules/zpty.c
> +++ b/Src/Modules/zpty.c
> @@ -534,11 +534,10 @@ static void
>  checkptycmd(Ptycmd cmd)
>  {
>      unsigned char c;
> -    int r;
>
>      if (cmd->read != -1 || cmd->fin)
>         return;
> -    if ((r = read(cmd->fd, &c, 1)) <= 0) {
> +    if (read(cmd->fd, &c, 1) <= 0) {
>         if (kill(cmd->pid, 0) < 0) {
>             cmd->fin = 1;
>             zclose(cmd->fd);
> diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
> index 54e6146ad..d7cdd76f7 100644
> --- a/Src/Zle/compcore.c
> +++ b/Src/Zle/compcore.c
> @@ -1176,7 +1176,7 @@ check_param(char *s, int set, int test)
>      if (found &&
>         p[1] != Inpar && p[1] != Inbrack && p[1] != Snull) {
>         /* This is a parameter expression, not $(...), $[...], $'...'. */
> -       char *b = p + 1, *e = b, *ie;
> +       char *b = p + 1, *e, *ie;
>         int br = 1, nest = 0;
>
>         if (*b == Inbrace) {
> @@ -1719,7 +1719,7 @@ set_comp_sep(void)
>         return 1;
>      owb = offs;
>      offs = soffs;
> -    if ((p = check_param(ns, 0, 1))) {
> +    if (check_param(ns, 0, 1)) {
>         for (p = ns; *p; p++)
>             if (*p == Dnull)
>                 *p = '"';
> diff --git a/Src/Zle/compctl.c b/Src/Zle/compctl.c
> index de3ccdfce..acd4b0435 100644
> --- a/Src/Zle/compctl.c
> +++ b/Src/Zle/compctl.c
> @@ -1462,8 +1462,6 @@ printcompctl(char *s, Compctl cc, int printflags, int ispat)
>
>         while (cc2) {
>             /* loop over conditions */
> -           c = cc2->cond;
> -
>             printf(" '");
>             for (c = cc2->cond; c;) {
>                 /* loop over or's */
> @@ -2886,7 +2884,7 @@ sep_comp_string(char *ss, char *s, int noffs)
>         return 1;
>      owb = offs;
>      offs = soffs;
> -    if ((p = check_param(ns, 0, 1))) {
> +    if (check_param(ns, 0, 1)) {
>         for (p = ns; *p; p++)
>             if (*p == Dnull)
>                 *p = '"';
> @@ -3609,8 +3607,7 @@ makecomplistflags(Compctl cc, char *s, int incmd, int compadd)
>                                         } else {
>                                             /* Otherwise ignore the path we *
>                                              * prepended to the pattern.    */
> -                                           while ((p2 = p3 =
> -                                                   (char *)ugetnode(l))) {
> +                                           while ((p3 = (char *) ugetnode(l))) {
>                                                 for (ns = sf1; *p3 && ns; p3++)
>                                                     if (*p3 == '/')
>                                                         ns--;
> diff --git a/Src/Zle/complist.c b/Src/Zle/complist.c
> index 21d0d6e60..6a4649795 100644
> --- a/Src/Zle/complist.c
> +++ b/Src/Zle/complist.c
> @@ -670,7 +670,7 @@ doiscol(int pos)
>  static int
>  clprintfmt(char *p, int ml)
>  {
> -    int cc = 0, i = 0, ask, beg;
> +    int cc = 0, i = 0, ask;
>
>      initiscol();
>
> @@ -696,7 +696,7 @@ clprintfmt(char *p, int ml)
>             chrlen--;
>             p++;
>         }
> -       if ((beg = !(cc % zterm_columns)))
> +       if (!(cc % zterm_columns))
>             ml++;
>         if (mscroll && !(cc % zterm_columns) &&
>             !--mrestlines && (ask = asklistscroll(ml)))
> @@ -1072,7 +1072,7 @@ static int
>  compprintfmt(char *fmt, int n, int dopr, int doesc, int ml, int *stop)
>  {
>      char *p, nc[2*DIGBUFSIZE + 12], nbuf[2*DIGBUFSIZE + 12];
> -    int l = 0, cc = 0, m, ask, beg, stat;
> +    int l = 0, cc = 0, m, beg, stat;
>
>      if ((stat = !fmt)) {
>         if (mlbeg >= 0) {
> @@ -1301,7 +1301,7 @@ compprintfmt(char *fmt, int n, int dopr, int doesc, int ml, int *stop)
>                     ml++;
>                      fputs(" \010", shout);
>                  }
> -               if (mscroll && beg && !--mrestlines && (ask = asklistscroll(ml))) {
> +               if (mscroll && beg && !--mrestlines && asklistscroll(ml)) {
>                     *stop = 1;
>                     if (stat && n)
>                         mfirstl = -1;
> @@ -1624,7 +1624,7 @@ compprintlist(int showall)
>                 q = p;
>                 while (n && i-- && !errflag) {
>                     wid = (g->widths ? g->widths[mc] : g->width);
> -                   if (!(m = *q)) {
> +                   if (!*q) {
>                         if (clprintm(g, NULL, mc, ml, (!i), wid))
>                             goto end;
>                         break;
> diff --git a/Src/Zle/compmatch.c b/Src/Zle/compmatch.c
> index bc82ff4d0..5b4f60588 100644
> --- a/Src/Zle/compmatch.c
> +++ b/Src/Zle/compmatch.c
> @@ -189,7 +189,7 @@ free_cline(Cline l)
>  Cline
>  cp_cline(Cline l, int deep)
>  {
> -    Cline r = NULL, *p = &r, t, lp = NULL;
> +    Cline r = NULL, *p = &r, t;
>
>      while (l) {
>         if ((t = freecl))
> @@ -203,7 +203,7 @@ cp_cline(Cline l, int deep)
>             if (t->suffix)
>                 t->suffix = cp_cline(t->suffix, 0);
>         }
> -       *p = lp = t;
> +       *p = t;
>         p = &(t->next);
>         l = l->next;
>      }
> diff --git a/Src/Zle/compresult.c b/Src/Zle/compresult.c
> index 45f01fef5..e3aa679ab 100644
> --- a/Src/Zle/compresult.c
> +++ b/Src/Zle/compresult.c
> @@ -965,10 +965,9 @@ do_single(Cmatch m)
>      int l, sr = 0, scs;
>      int havesuff = 0;
>      int partest = (m->ripre || ((m->flags & CMF_ISPAR) && parpre));
> -    char *str = m->orig, *ppre = m->ppre, *psuf = m->psuf, *prpre = m->prpre;
> +    char *str = m->orig, *psuf = m->psuf, *prpre = m->prpre;
>
>      if (!prpre) prpre = "";
> -    if (!ppre) ppre = "";
>      if (!psuf) psuf = "";
>
>      fixsuffix();
> @@ -1138,7 +1137,6 @@ do_single(Cmatch m)
>             /*{{*/
>             /* Otherwise, add a `,' suffix, and let `}' remove it. */
>             zlemetacs = scs;
> -           havesuff = 1;
>             inststrlen(",", 1, 1);
>             minfo.insc++;
>             makesuffix(1);
> @@ -2123,7 +2121,7 @@ printlist(int over, CLPrintFunc printm, int showall)
>                 q = p;
>                 while (n && i--) {
>                     wid = (g->widths ? g->widths[mc] : g->width);
> -                   if (!(m = *q)) {
> +                   if (!*q) {
>                         printm(g, NULL, mc, ml, (!i), wid);
>                         break;
>                     }
> diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
> index 74370d0fc..58daec01a 100644
> --- a/Src/Zle/computil.c
> +++ b/Src/Zle/computil.c
> @@ -3481,7 +3481,6 @@ cv_parse_word(Cvdef d)
>                          more[-1] = sav;
>                      } else {
>                          zaddlinknode(state.vals, tricat(arg, compsuffix, ""));
> -                        nosfx = 1;
>                      }
>                  } else
>                      zaddlinknode(state.vals, ztrdup(""));
> @@ -4694,10 +4693,9 @@ cfp_opt_pats(char **pats, char *matcher)
>                     for (s = add; *s && !idigit(*s); s++);
>                     *s = '\0';
>                 } else if (*q == '[') {
> -                   int not;
>                     char *x = ++q;
>
> -                   if ((not = (*x == '!' || *x == '^')))
> +                   if (*x == '!' || *x == '^')
>                         x++;
>                     for (; *x; x++) {
>                         if (x[1] == '-' && x[2]) {
> diff --git a/Src/Zle/zle_keymap.c b/Src/Zle/zle_keymap.c
> index e26895a2b..e28ff6f35 100644
> --- a/Src/Zle/zle_keymap.c
> +++ b/Src/Zle/zle_keymap.c
> @@ -1778,7 +1778,6 @@ getkeycmd(void)
>      if(!func) {
>         if (++hops == 20) {
>             zerr("string inserting another one too many times");
> -           hops = 0;
>             return NULL;
>         }
>         ungetbytes_unmeta(str, strlen(str));
> diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c
> index 77fa5fbe4..a1aa9b35f 100644
> --- a/Src/Zle/zle_tricky.c
> +++ b/Src/Zle/zle_tricky.c
> @@ -538,7 +538,7 @@ parambeg(char *s)
>          * This is really a parameter expression (not $(...) or $[...]
>          * or $'...').
>          */
> -       char *b = p + 1, *e = b;
> +       char *b = p + 1, *e;
>         int n = 0, br = 1;
>
>         if (*b == Inbrace) {
> @@ -1655,7 +1655,7 @@ get_comp_string(void)
>              * so start at the beginning of the line and continue
>              * until cspos.
>              */
> -           wptr = cptr = zlemetaline;
> +           wptr = zlemetaline;
>             for (;;) {
>                 cptr = itype_end(wptr, IIDENT, 0);
>                 if (cptr == wptr) {
> @@ -1678,7 +1678,7 @@ get_comp_string(void)
>             char *sqbr = zlemetaline + wb - 1, *cptr, *wptr;
>
>             /* Need to search forward for word characters */
> -           cptr = wptr = zlemetaline;
> +           wptr = zlemetaline;
>             for (;;) {
>                 cptr = itype_end(wptr, IIDENT, 0);
>                 if (cptr == wptr) {
> @@ -1706,7 +1706,7 @@ get_comp_string(void)
>      }
>      /* This variable will hold the current word in quoted form. */
>      offs = zlemetacs - wb;
> -    if ((p = parambeg(s))) {
> +    if (parambeg(s)) {
>         for (p = s; *p; p++)
>             if (*p == Dnull)
>                 *p = '"';
> diff --git a/Src/builtin.c b/Src/builtin.c
> index 706191832..d2326a909 100644
> --- a/Src/builtin.c
> +++ b/Src/builtin.c
> @@ -6591,8 +6591,7 @@ bin_read(char *name, char **args, Options ops, UNUSED(int func))
>      if (OPT_ISSET(ops,'t')) {
>         zlong timeout = 0;
>         if (OPT_HASARG(ops,'t')) {
> -           mnumber mn = zero_mnumber;
> -           mn = matheval(OPT_ARG(ops,'t'));
> +           mnumber mn = matheval(OPT_ARG(ops,'t'));
>             if (errflag)
>                 return 1;
>             if (mn.type == MN_FLOAT) {
> diff --git a/Src/exec.c b/Src/exec.c
> index 2c730b910..933a28233 100644
> --- a/Src/exec.c
> +++ b/Src/exec.c
> @@ -693,7 +693,7 @@ search_defpath(char *cmd, char *pbuf, int plen)
>  {
>      char *ps = DEFAULT_PATH, *pe = NULL, *s;
>
> -    for (ps = DEFAULT_PATH; ps; ps = pe ? pe+1 : NULL) {
> +    for (; ps; ps = pe ? pe+1 : NULL) {
>         pe = strchr(ps, ':');
>         if (*ps == '/') {
>             s = pbuf;
> @@ -1500,9 +1500,9 @@ execlist(Estate state, int dont_change_job, int exiting)
>             case WC_SUBLIST_AND:
>                 /* If the return code is non-zero, we skip pipelines until *
>                  * we find a sublist followed by ORNEXT.                   */
> -               if ((ret = ((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_SIMPLE) ?
> +               if (((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_SIMPLE) ?
>                             execsimple(state) :
> -                           execpline(state, code, Z_SYNC, 0))) || breaks) {
> +                           execpline(state, code, Z_SYNC, 0)) || breaks) {
>                     state->pc = next;
>                     code = *state->pc++;
>                     next = state->pc + WC_SUBLIST_SKIP(code);
> @@ -1533,7 +1533,7 @@ execlist(Estate state, int dont_change_job, int exiting)
>             case WC_SUBLIST_OR:
>                 /* If the return code is zero, we skip pipelines until *
>                  * we find a sublist followed by ANDNEXT.              */
> -               if (!(ret = ((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_SIMPLE) ?
> +               if (!(((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_SIMPLE) ?
>                              execsimple(state) :
>                              execpline(state, code, Z_SYNC, 0))) || breaks) {
>                     state->pc = next;
> @@ -2689,11 +2689,8 @@ execsubst(LinkList strs)
>  {
>      if (strs) {
>         prefork(strs, esprefork, NULL);
> -       if (esglob && !errflag) {
> -           LinkList ostrs = strs;
> +       if (esglob && !errflag)
>             globlist(strs, 0);
> -           strs = ostrs;
> -       }
>      }
>  }
>
> @@ -5240,7 +5237,7 @@ static int
>  execarith(Estate state, UNUSED(int do_exec))
>  {
>      char *e;
> -    mnumber val = zero_mnumber;
> +    mnumber val;
>      int htok = 0;
>
>      if (isset(XTRACE)) {
> diff --git a/Src/glob.c b/Src/glob.c
> index 7511c1c15..e9967cad1 100644
> --- a/Src/glob.c
> +++ b/Src/glob.c
> @@ -849,7 +849,6 @@ qgetmodespec(char **s)
>      if ((c = *p) == '=' || c == Equals || c == '+' || c == '-' ||
>         c == '?' || c == Quest || (c >= '0' && c <= '7')) {
>         end = 0;
> -       c = 0;
>      } else {
>         end = (c == '<' ? '>' :
>                (c == '[' ? ']' :
> diff --git a/Src/init.c b/Src/init.c
> index a422036c7..28bff0763 100644
> --- a/Src/init.c
> +++ b/Src/init.c
> @@ -548,10 +548,8 @@ parseopts(char *nam, char ***argvp, char *new_opts, char **cmdp,
>      }
>   doneargv:
>      *argvp = argv;
> -    if (emulate_required) {
> +    if (emulate_required)
>         parseopts_setemulate(top_emulation, flags);
> -       emulate_required = 0;
> -    }
>      return 0;
>  }
>
> diff --git a/Src/params.c b/Src/params.c
> index 5fe7107af..1b3f10b46 100644
> --- a/Src/params.c
> +++ b/Src/params.c
> @@ -1131,7 +1131,6 @@ createparam(char *name, int flags)
>
>             pm = oldpm;
>             pm->base = pm->width = 0;
> -           oldpm = pm->old;
>         } else {
>             pm = (Param) zshcalloc(sizeof *pm);
>             if ((pm->old = oldpm)) {
> diff --git a/Src/prompt.c b/Src/prompt.c
> index 8325bfe2c..7db074da8 100644
> --- a/Src/prompt.c
> +++ b/Src/prompt.c
> @@ -1956,7 +1956,7 @@ truecolor_terminal(void)
>  mod_export zattr
>  match_colour(const char **teststrp, int is_fg, int colour)
>  {
> -    int shft, named = 0, tc;
> +    int shft, tc;
>      zattr on;
>
>      if (is_fg) {
> @@ -1997,7 +1997,7 @@ match_colour(const char **teststrp, int is_fg, int colour)
>             } else if (colour <= -2) {
>                 return TXT_ERROR;
>             }
> -       } else if ((named = ialpha(**teststrp))) {
> +       } else if (ialpha(**teststrp)) {
>             colour = match_named_colour(teststrp);
>             if (colour == 8) /* default */
>                 return 0;
> diff --git a/Src/sort.c b/Src/sort.c
> index ce2b4bbc3..224741b00 100644
> --- a/Src/sort.c
> +++ b/Src/sort.c
> @@ -255,7 +255,7 @@ strmetasort(char **array, int sortwhat, int *unmetalenp)
>      for (arrptr = array, sortptrarrptr = sortptrarr, sortarrptr = sortarr;
>          *arrptr; arrptr++, sortptrarrptr++, sortarrptr++) {
>         char *metaptr;
> -       int needlen, needalloc;
> +       int needlen;
>         *sortptrarrptr = sortarrptr;
>         sortarrptr->orig = *arrptr;
>
> @@ -286,8 +286,7 @@ strmetasort(char **array, int sortwhat, int *unmetalenp)
>          * Either we're going to need to copy it to transform it,
>          * or we need to unmetafy it.
>          */
> -       if ((needalloc = (sortwhat &
> -                         (SORTIT_IGNORING_CASE|SORTIT_IGNORING_BACKSLASHES)))
> +       if ((sortwhat & (SORTIT_IGNORING_CASE|SORTIT_IGNORING_BACKSLASHES))
>             || *metaptr == Meta) {
>             char *s, *t, *src = *arrptr, *dst;
>             int len;
> diff --git a/Src/subst.c b/Src/subst.c
> index 5f8436e28..16a3cb055 100644
> --- a/Src/subst.c
> +++ b/Src/subst.c
> @@ -242,7 +242,7 @@ stringsubst(LinkList list, LinkNode node, int pf_flags, int *ret_flags,
>      char *str  = str3, c;
>
>      while (!errflag && (c = *str)) {
> -       if (((c = *str) == Inang || c == OutangProc ||
> +       if ((c == Inang || c == OutangProc ||
>              (str == str3 && c == Equals))
>             && str[1] == Inpar) {
>             char *subst, *rest, *snew, *sptr;
> @@ -2348,7 +2348,6 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
>                         postmul = untok_and_escape(s + arglen, escapes,
>                                                    tok_arg);
>                     *t = sav;
> -                   sav = *s;
>                     s = t + arglen;
>                     /* again, continue only if another start delimiter */
>                     if (memcmp(del0, s, dellen)) {
>


-- 
Mikael Magnusson




Messages sorted by: Reverse Date, Date, Thread, Author