Re: [Xen-devel] [PATCH] tools/xl: fix uninitialized variable in xl_vdispl
On 3/13/18 3:03 AM, Jan Beulich wrote: On 13.03.18 at 05:43,wrote: >> The code added in 7a48622a78a0b452e8afa55b8442c958abd226a7 could use rc >> uninitialized in main_vdisplattach(). >> >> Signed-off-by: Doug Goldstein >> --- >> CC: Oleksandr Grytsov >> --- >> tools/xl/xl_vdispl.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Please trim your Cc list - I've removed all individuals here, and I > don't see why you've copied all REST maintainers when this is > clearly a pure tool stack change. > > Jan > Yea it was a mistake on my part. Submitting patches late at night caused me to add a -f to get_maintainer.pl -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] tools/xl: fix uninitialized variable in xl_vdispl
On Tue, Mar 13, 2018 at 10:56:35AM +, George Dunlap wrote: > On 03/13/2018 04:43 AM, Doug Goldstein wrote: > > The code added in 7a48622a78a0b452e8afa55b8442c958abd226a7 could use rc > > uninitialized in main_vdisplattach(). > > > > Signed-off-by: Doug Goldstein> > It looks like that was designed on purpose to use the uninitialized > warnings to catch paths where the rc wasn't specifically set. > > The only path where it's not set is if dryrun_only is true; and in that > case, we probably actually want it to succeed, not fail. > > No matter what, rc = 0 should be added to the dryrun_only path. If we > want to make ERROR_FAIL the default, then the various places rc is set > to ERROR_FAIL should be removed. Yeah, CODING_STYLE dictates rc to not be initialised. Setting rc = 0 in the appropriate place is the right solution. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] tools/xl: fix uninitialized variable in xl_vdispl
On 03/13/2018 04:43 AM, Doug Goldstein wrote: > The code added in 7a48622a78a0b452e8afa55b8442c958abd226a7 could use rc > uninitialized in main_vdisplattach(). > > Signed-off-by: Doug GoldsteinIt looks like that was designed on purpose to use the uninitialized warnings to catch paths where the rc wasn't specifically set. The only path where it's not set is if dryrun_only is true; and in that case, we probably actually want it to succeed, not fail. No matter what, rc = 0 should be added to the dryrun_only path. If we want to make ERROR_FAIL the default, then the various places rc is set to ERROR_FAIL should be removed. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] tools/xl: fix uninitialized variable in xl_vdispl
On Tue, Mar 13, 2018 at 10:03 AM, Jan Beulichwrote: > >>> On 13.03.18 at 05:43, wrote: > > The code added in 7a48622a78a0b452e8afa55b8442c958abd226a7 could use rc > > uninitialized in main_vdisplattach(). > > > > Signed-off-by: Doug Goldstein > > --- > > CC: Oleksandr Grytsov > > --- > > tools/xl/xl_vdispl.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Please trim your Cc list - I've removed all individuals here, and I > don't see why you've copied all REST maintainers when this is > clearly a pure tool stack change. > > Jan > > > ___ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel > Hi, Doug, Thanks for pointing it out. The implementation is done according to CODING_STYLE document (see ERROR HANDLING) which requests to define return value rc uninitialized. The only path where rc is uninitialized it is when dryrun_only set to true. So, proper fix should be: if (dryrun_only) { char *json = libxl_device_vdispl_to_json(ctx, ); printf("vdispl: %s\n", json); free(json); + rc = 0; goto out; } -- Best Regards, Oleksandr Grytsov. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] tools/xl: fix uninitialized variable in xl_vdispl
>>> On 13.03.18 at 05:43,wrote: > The code added in 7a48622a78a0b452e8afa55b8442c958abd226a7 could use rc > uninitialized in main_vdisplattach(). > > Signed-off-by: Doug Goldstein > --- > CC: Oleksandr Grytsov > --- > tools/xl/xl_vdispl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Please trim your Cc list - I've removed all individuals here, and I don't see why you've copied all REST maintainers when this is clearly a pure tool stack change. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel