On Thu, 9 Mar 2023 at 02:09, joshua stein <j...@jcs.org> wrote:
> 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?
ssh uses this pattern a lot, and I agree with millert that it's not
worth changing.
char *thing = NULL;
[lots of stuff that might set thing]
if (maybe()) {
free(thing);
thing = something;
}
[more stuff]
free(thing)
We actually went through and changed all the "if (thing) free(thing)"
instances to drop the "if" some time ago.
> grp == NULL fatal()s, so remove the ternary operations that will
> never be the conditionals they aspire to be.
That's true in OpenBSD, but not in -portable where the check is a
debug only. That said, there's already a diff in this code block
that'll stop future syncs from applying cleanly so I don't mind either
way.
> + if ((r = encode_dest_constraint_hop(b, &dc->from)) != 0 ||
> + (r = encode_dest_constraint_hop(b, &dc->to)) != 0 ||
I'll wait for djm to comment on this one.
> + if ((negate = (attrib[0] == '!')))
This seems to be one too many parens? ie
if (negate = (attrib[0] == '!'))
> - 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)
also djm.
--
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
