Re: [Xen-devel] [PATCH] tools/xl: fix uninitialized variable in xl_vdispl

2018-03-13 Thread Doug Goldstein
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

2018-03-13 Thread Wei Liu
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

2018-03-13 Thread George Dunlap
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.

 -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

2018-03-13 Thread Oleksandr Grytsov
On Tue, Mar 13, 2018 at 10: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
>
>
> ___
> 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

2018-03-13 Thread Jan Beulich
>>> 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