| Message ID | f46a05695dc1dcab6925a1faf3cdb80c0140969c.1716807712.git.echaudro@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,1/7] Coverity: Fix Coverity `Unintentional integer overflow` reports. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 5/27/24 13:01, Eelco Chaudron wrote: > Coverity was flagged that the uninitialized output variable was used > in the ctl_context_init_command() function. This patch initializes > the variable. > > Fixes: 07ff77ccb82a ("db-ctl-base: Make common database command code into library.") > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> > --- > 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 3a8068b12..4c70d20ce 100644 > --- a/lib/db-ctl-base.c > +++ b/lib/db-ctl-base.c > @@ -2656,6 +2656,7 @@ ctl_context_init(struct ctl_context *ctx, struct ctl_command *command, > struct ovsdb_symbol_table *symtab, > void (*invalidate_cache_cb)(struct ctl_context *)) > { > + ds_init(&ctx->output); > if (command) { > ctl_context_init_command(ctx, command, false); > } I don't see this string to be destroyed anywhere, should this be added to the context_done() ? I see it's being swapped around with command output, but it's hard to track. Best regards, Ilya Maximets.
On 27 May 2024, at 16:50, Ilya Maximets wrote: > On 5/27/24 13:01, Eelco Chaudron wrote: >> Coverity was flagged that the uninitialized output variable was used >> in the ctl_context_init_command() function. This patch initializes >> the variable. >> >> Fixes: 07ff77ccb82a ("db-ctl-base: Make common database command code into library.") >> Signed-off-by: Eelco Chaudron <echaudro@redhat.com> >> --- >> 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 3a8068b12..4c70d20ce 100644 >> --- a/lib/db-ctl-base.c >> +++ b/lib/db-ctl-base.c >> @@ -2656,6 +2656,7 @@ ctl_context_init(struct ctl_context *ctx, struct ctl_command *command, >> struct ovsdb_symbol_table *symtab, >> void (*invalidate_cache_cb)(struct ctl_context *)) >> { >> + ds_init(&ctx->output); >> if (command) { >> ctl_context_init_command(ctx, command, false); >> } > > I don't see this string to be destroyed anywhere, should this be > added to the context_done() ? I see it's being swapped around with > command output, but it's hard to track. ACK, I had the same impression. I looked at the existing use cases, and due to the swap (and back) it seems ok. But anyhow I did some tests with adding ds_destroy() to ctl_context_done() just to be sure. Will send a v2 soon. //Eelco
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c index 3a8068b12..4c70d20ce 100644 --- a/lib/db-ctl-base.c +++ b/lib/db-ctl-base.c @@ -2656,6 +2656,7 @@ ctl_context_init(struct ctl_context *ctx, struct ctl_command *command, struct ovsdb_symbol_table *symtab, void (*invalidate_cache_cb)(struct ctl_context *)) { + ds_init(&ctx->output); if (command) { ctl_context_init_command(ctx, command, false); }
Coverity was flagged that the uninitialized output variable was used in the ctl_context_init_command() function. This patch initializes the variable. Fixes: 07ff77ccb82a ("db-ctl-base: Make common database command code into library.") Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- lib/db-ctl-base.c | 1 + 1 file changed, 1 insertion(+)