ssh nits

[joshua stein]

cppcheck found these, are they worth fixing?


In the non-fail case, done is set to NULL and then free()d.  
free(NULL) is legal but maybe worth removing?

diff --git usr.bin/ssh/scp.c usr.bin/ssh/scp.c
index f0f09bba623..acb7bd8a8a1 100644
--- usr.bin/ssh/scp.c
+++ usr.bin/ssh/scp.c
@@ -935,19 +935,21 @@ brace_expand(const char *pattern, char ***patternsp, 
size_t *npatternsp)
        *npatternsp = ndone;
        done = NULL;
        ndone = 0;
        ret = 0;
  fail:
        for (i = 0; i < nactive; i++)
                free(active[i]);
        free(active);
-       for (i = 0; i < ndone; i++)
-               free(done[i]);
-       free(done);
+       if (done) {
+               for (i = 0; i < ndone; i++)
+                       free(done[i]);
+               free(done);
+       }
        return ret;
 }
 
 static struct sftp_conn *
 do_sftp_connect(char *host, char *user, int port, char *sftp_direct,
    int *reminp, int *remoutp, int *pidp)
 {
        if (sftp_direct == NULL) {


grp == NULL fatal()s, so remove the ternary operations that will 
never be the conditionals they aspire to be.

diff --git usr.bin/ssh/sshpty.c usr.bin/ssh/sshpty.c
index faf8960cfb5..690263a8cf3 100644
--- usr.bin/ssh/sshpty.c
+++ usr.bin/ssh/sshpty.c
@@ -143,8 +143,8 @@ pty_setowner(struct passwd *pw, const char *tty)
        grp = getgrnam("tty");
        if (grp == NULL)
                fatal("no tty group");
-       gid = (grp != NULL) ? grp->gr_gid : pw->pw_gid;
-       mode = (grp != NULL) ? 0620 : 0600;
+       gid = grp->gr_gid;
+       mode = 0620;
 
        /*
         * Change owner and mode of the tty as required.


These parentheses checking the result of an assignment were 
confusing, so move them.

diff --git usr.bin/ssh/authfd.c usr.bin/ssh/authfd.c
index 4b81b385637..05011f8c5c9 100644
--- usr.bin/ssh/authfd.c
+++ usr.bin/ssh/authfd.c
@@ -489,8 +489,8 @@ encode_dest_constraint(struct sshbuf *m, const struct 
dest_constraint *dc)
 
        if ((b = sshbuf_new()) == NULL)
                return SSH_ERR_ALLOC_FAIL;
-       if ((r = encode_dest_constraint_hop(b, &dc->from) != 0) ||
-           (r = encode_dest_constraint_hop(b, &dc->to) != 0) ||
+       if ((r = encode_dest_constraint_hop(b, &dc->from)) != 0 ||
+           (r = encode_dest_constraint_hop(b, &dc->to)) != 0 ||
            (r = sshbuf_put_string(b, NULL, 0)) != 0) /* reserved */
                goto out;
        if ((r = sshbuf_put_stringb(m, b)) != 0)
diff --git usr.bin/ssh/readconf.c usr.bin/ssh/readconf.c
index e9d3a756896..81456c9b6d3 100644
--- usr.bin/ssh/readconf.c
+++ usr.bin/ssh/readconf.c
@@ -602,7 +602,7 @@ match_cfg_line(Options *options, char **condition, struct 
passwd *pw,
                }
                arg = criteria = NULL;
                this_result = 1;
-               if ((negate = attrib[0] == '!'))
+               if ((negate = (attrib[0] == '!')))
                        attrib++;
                /* Criterion "all" has no argument and must appear alone */
                if (strcasecmp(attrib, "all") == 0) {
diff --git usr.bin/ssh/ssh-agent.c usr.bin/ssh/ssh-agent.c
index 0e4d7f675ab..de1cdb049a2 100644
--- usr.bin/ssh/ssh-agent.c
+++ usr.bin/ssh/ssh-agent.c
@@ -1010,8 +1010,8 @@ parse_dest_constraint(struct sshbuf *m, struct 
dest_constraint *dc)
                error_fr(r, "parse");
                goto out;
        }
-       if ((r = parse_dest_constraint_hop(frombuf, &dc->from) != 0) ||
-           (r = parse_dest_constraint_hop(tobuf, &dc->to) != 0))
+       if ((r = parse_dest_constraint_hop(frombuf, &dc->from)) != 0 ||
+           (r = parse_dest_constraint_hop(tobuf, &dc->to)) != 0)
                goto out; /* already logged */
        if (elen != 0) {
                error_f("unsupported extensions (len %zu)", elen);