possible deadcodes in sources

Eldon Koyle esk-openssh at esk.cs.usu.edu
Tue Sep 9 05:51:56 EST 2014


On  Sep 08 13:01-0600, Eldon Koyle wrote:
<snip>
> In monitor_wrap.c, it looks like your test might be assuming that two
> calls to dup() will return the same value, which is definitely not the
> case.

Looking more closely, the second if case would never be reached unless
tmp2 has been set elsewhere (or is never initialized), in which case
closing it might in fact be a bug and not just a coverage issue.

<snip>

-- 
Eldon Koyle
Information Technology
Utah State University
-- 
Liberty don't work as good in practice as it does in speeches.
		-- The Best of Will Rogers

> On  Sep 08 20:31+0200, Petr Lautrbach wrote:
> > Hello,
> > 
> > we've run a coverity scan on the openssh sources and it found several
> > issues. Although the scan was run on patched rhel sources, some results are applicable to vanilla sources 
> > too.
> > 
> >  * servconf.c:1458:dead_error_line – Execution cannot reach this statement "*intptr = *intptr + 1;"
> > 
> > --- a/servconf.c
> > +++ b/servconf.c
> > @@ -1451,12 +1451,8 @@ process_server_config_line(ServerOptions *options, char *line,
> >                 if (!arg || *arg == '\0')
> >                         fatal("%s line %d: missing file name.",
> >                             filename, linenum);
> > -               if (*activep && *charptr == NULL) {
> > +               if (*activep && *charptr == NULL)
> >                         *charptr = tilde_expand_filename(arg, getuid());
> > -                       /* increase optional counter */
> > -                       if (intptr != NULL)
> > -                               *intptr = *intptr + 1;
> > -               }
> >                 break;
> >  
> >         case sClientAliveInterval:
> > 
> > 
> > 
> >  * monitor_wrap.c:720:dead_error_line – Execution cannot reach this statement "close(tmp2);".o
> > 
> > --- a/monitor_wrap.c
> > +++ b/monitor_wrap.c
> > @@ -714,10 +714,8 @@ mm_pty_allocate(int *ptyfd, int *ttyfd, char *namebuf, size_t namebuflen)
> >         if ((tmp1 = dup(pmonitor->m_recvfd)) == -1 ||
> >             (tmp2 = dup(pmonitor->m_recvfd)) == -1) {
> >                 error("%s: cannot allocate fds for pty", __func__);
> > -               if (tmp1 > 0)
> > +               if (tmp1 > -1)
> >                         close(tmp1);
> > -               if (tmp2 > 0)
> > -                       close(tmp2);
> >                 return 0;
> >         }
> >         close(tmp1);
> > 
> > 
> >  * sshkey.c:1321:dead_error_line – Execution cannot reach this statement "break;".
> > 
> > code:
> > 		retval = 0;
> > /*XXXX*/
> > 		sshkey_free(k);
> > 		if (retval != 0)
> > 			break;
> > 
> > XXXX here probably means fix in future, but the last two lines seem to be functionless
> > 
> > 
> > 
> >  * clientloop.c:2087:dead_error_line – Execution cannot reach this expression "81" inside statement "packet_start((success ? 81 ...". 
> > 
> > 
> > 
> > 
> > I hope that it makes sense.
> > 
> > -- 
> > Petr Lautrbach
> > _______________________________________________
> > openssh-unix-dev mailing list
> > openssh-unix-dev at mindrot.org
> > https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
> 
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev at mindrot.org
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


More information about the openssh-unix-dev mailing list