On Mon, Sep 08, 2014 at 01:01:59PM -0600, Eldon Koyle wrote:
> I think coverity is probably wrong on some of these. Code coverage
> testing is a hard problem, and it is not uncommon for the tools to get
> it wrong.
I'll try to explain why I think that coverity is right.
> > * 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;
> >
The original code:
848 int cmdline = 0, *intptr, value, value2, n, port;
...
865 intptr = NULL;
...
886 switch (opcode) {
...
1448 case sAuthorizedPrincipalsFile:
1449 charptr = &options->authorized_principals_file;
1450 arg = strdelim(&cp);
1451 if (!arg || *arg == '\0')
1452 fatal("%s line %d: missing file name.",
1453 filename, linenum);
1454 if (*activep && *charptr == NULL) {
1455 *charptr = tilde_expand_filename(arg,
getuid());
1456 /* increase optional counter */
1457 if (intptr != NULL)
1458 *intptr = *intptr + 1;
1459 }
1460 break;
- intptr is set to NULL, it's not changed and then is compared if it's not NULL
- intptr is used as a reference to options->num_host_key_files when opcode is sHostKeyFile. While
the it's possible to have more then one hostkey, the authorizedprincipalsfile can be only only. I guess
that lines 1456 - 1458 were jsut cut&pasted from sHostKeyFile block.
> >
> > * 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);
711 int success = 0, tmp1 = -1, tmp2 = -1;
712
713 /* Kludge: ensure there are fds free to receive the pty/tty */
714 if ((tmp1 = dup(pmonitor->m_recvfd)) == -1 ||
715 (tmp2 = dup(pmonitor->m_recvfd)) == -1) {
716 error("%s: cannot allocate fds for pty", __func__);
717 if (tmp1 > 0)
718 close(tmp1);
719 if (tmp2 > 0)
720 close(tmp2);
721 return 0;
722 }
- when first dup(pmonitor->m_recvfd) fails, tmp1 == -1 and tmp2 == -1
- when second dup(pmonitor->m_recvfd) fails, tmp1 is the first unused fd which can be 0 or more and this should
be closed while tmp2 == -1 and there's no action needed
> >
> > * 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
1180 int retval = SSH_ERR_INVALID_FORMAT;
...
1317 retval = 0;
1318 /*XXXX*/
1319 sshkey_free(k);
1320 if (retval != 0)
1321 break;
> > * clientloop.c:2087:dead_error_line – Execution cannot reach this expression "81" inside statement "packet_start((success ? 81 ...".
> >
2075 static void
2076 client_input_global_request(int type, u_int32_t seq, void *ctxt)
2077 {
2078 char *rtype;
2079 int want_reply;
2080 int success = 0;
2081
2082 rtype = packet_get_string(NULL);
2083 want_reply = packet_get_char();
2084 debug("client_input_global_request: rtype %s want_reply %d",
2085 rtype, want_reply);
2086 if (want_reply) {
2087 packet_start(success ?
2088 SSH2_MSG_REQUEST_SUCCESS : SSH2_MSG_REQUEST_FAILURE);
2089 packet_send();
2090 packet_write_wait();
2091 }
2092 free(rtype);
2093 }
Here I'm not sure if there's missing code or it should simple sent always SSH2_MSG_REQUEST_FAILURE
--
Petr Lautrbach
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
