On 6/1/26 4:26 PM, Ilya Maximets wrote: > On 5/19/26 3:19 PM, Timothy Redaelli via dev wrote: >> When local_options are present but no command name follows, >> the early return leaked the previously allocated commands array. >> >> Free commands before returning the error string. >> >> Found by OpenScanHub Coverity (RESOURCE_LEAK). >> Signed-off-by: Timothy Redaelli <[email protected]> >> --- >> lib/db-ctl-base.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c >> index bd3a21dd9..67ef0215e 100644 >> --- a/lib/db-ctl-base.c >> +++ b/lib/db-ctl-base.c >> @@ -2480,6 +2480,7 @@ ctl_parse_commands(int argc, char *argv[], struct >> shash *local_options, >> >> n_commands++; >> } else if (!shash_is_empty(local_options)) { >> + free(commands); >> return xstrdup("missing command name (use --help for >> help)"); > > Hmm. This doesn't look right. The parse_command() consumes local_options, > so they are always empty if we ever tried to parse any command. So, if we > are taking this branch, then it means that commands were never allocated. > Besides, if we have commands allocated, then we need to destroy them before > freeing the holding array. Or am I missing something here? > > Best regards, Ilya Maximets.
You're right, I missed that parse_command() swaps local_options into command->options as the very first thing it does, before any error can occur. So once we have tried to parse any command, successfully or not, local_options is empty, and this branch can only be taken before any command was ever parsed, i.e. with commands == NULL. The added free() is a no-op there, so no harm done, but it's misleading. Please, drop this patch; I'll mark the report as a false positive in OpenScanHub. Thanks for catching this! _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
