* [PATCH] libxl: init_acpi_config should return rc in exit path
@ 2016-12-14 11:44 Wei Liu
2016-12-14 11:49 ` Andrew Cooper
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Wei Liu @ 2016-12-14 11:44 UTC (permalink / raw)
To: Xen-devel; +Cc: Ian Jackson, Wei Liu
... otherwise it returns 0 even if the function fails.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
This should be backported to 4.8.
---
tools/libxl/libxl_x86_acpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_x86_acpi.c b/tools/libxl/libxl_x86_acpi.c
index 686ac8e..6cf0c30 100644
--- a/tools/libxl/libxl_x86_acpi.c
+++ b/tools/libxl/libxl_x86_acpi.c
@@ -154,7 +154,7 @@ static int init_acpi_config(libxl__gc *gc,
config->acpi_revision = 5;
out:
- return 0;
+ return rc;
}
int libxl__dom_load_acpi(libxl__gc *gc,
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] libxl: init_acpi_config should return rc in exit path
2016-12-14 11:44 [PATCH] libxl: init_acpi_config should return rc in exit path Wei Liu
@ 2016-12-14 11:49 ` Andrew Cooper
2016-12-14 12:28 ` Wei Liu
2016-12-16 15:10 ` Boris Ostrovsky
2016-12-16 11:05 ` Ian Jackson
2016-12-16 16:23 ` Wei Liu
2 siblings, 2 replies; 12+ messages in thread
From: Andrew Cooper @ 2016-12-14 11:49 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Ian Jackson
[-- Attachment #1.1: Type: text/plain, Size: 784 bytes --]
On 14/12/16 11:44, Wei Liu wrote:
> ... otherwise it returns 0 even if the function fails.
Coverity-ID: 1397121
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>
> This should be backported to 4.8.
> ---
> tools/libxl/libxl_x86_acpi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/libxl/libxl_x86_acpi.c b/tools/libxl/libxl_x86_acpi.c
> index 686ac8e..6cf0c30 100644
> --- a/tools/libxl/libxl_x86_acpi.c
> +++ b/tools/libxl/libxl_x86_acpi.c
> @@ -154,7 +154,7 @@ static int init_acpi_config(libxl__gc *gc,
> config->acpi_revision = 5;
>
> out:
> - return 0;
> + return rc;
> }
>
> int libxl__dom_load_acpi(libxl__gc *gc,
[-- Attachment #1.2: Type: text/html, Size: 1732 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] libxl: init_acpi_config should return rc in exit path
2016-12-14 11:49 ` Andrew Cooper
@ 2016-12-14 12:28 ` Wei Liu
2016-12-16 15:10 ` Boris Ostrovsky
1 sibling, 0 replies; 12+ messages in thread
From: Wei Liu @ 2016-12-14 12:28 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Ian Jackson
On Wed, Dec 14, 2016 at 11:49:34AM +0000, Andrew Cooper wrote:
> On 14/12/16 11:44, Wei Liu wrote:
> > ... otherwise it returns 0 even if the function fails.
>
> Coverity-ID: 1397121
>
Does it work this way? Coverity does lead to the discovery of this bug,
but this CID doesn't reveal the bug directly.
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> > ---
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> >
> > This should be backported to 4.8.
> > ---
> > tools/libxl/libxl_x86_acpi.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/libxl/libxl_x86_acpi.c b/tools/libxl/libxl_x86_acpi.c
> > index 686ac8e..6cf0c30 100644
> > --- a/tools/libxl/libxl_x86_acpi.c
> > +++ b/tools/libxl/libxl_x86_acpi.c
> > @@ -154,7 +154,7 @@ static int init_acpi_config(libxl__gc *gc,
> > config->acpi_revision = 5;
> >
> > out:
> > - return 0;
> > + return rc;
> > }
> >
> > int libxl__dom_load_acpi(libxl__gc *gc,
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] libxl: init_acpi_config should return rc in exit path
2016-12-14 11:49 ` Andrew Cooper
2016-12-14 12:28 ` Wei Liu
@ 2016-12-16 15:10 ` Boris Ostrovsky
2016-12-16 15:16 ` Wei Liu
1 sibling, 1 reply; 12+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 15:10 UTC (permalink / raw)
To: Andrew Cooper, Wei Liu, Xen-devel; +Cc: Ian Jackson
On 12/14/2016 06:49 AM, Andrew Cooper wrote:
> On 14/12/16 11:44, Wei Liu wrote:
>> ... otherwise it returns 0 even if the function fails.
>
> Coverity-ID: 1397121
>
>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
>> ---
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>>
>> This should be backported to 4.8.
>> ---
>> tools/libxl/libxl_x86_acpi.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/libxl/libxl_x86_acpi.c b/tools/libxl/libxl_x86_acpi.c
>> index 686ac8e..6cf0c30 100644
>> --- a/tools/libxl/libxl_x86_acpi.c
>> +++ b/tools/libxl/libxl_x86_acpi.c
>> @@ -154,7 +154,7 @@ static int init_acpi_config(libxl__gc *gc,
>> config->acpi_revision = 5;
>>
>> out:
>> - return 0;
>> + return rc;
This breaks init_acpi_config(): xc_domain_getinfo() at the top of
returns a positive number (number of domains).
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] libxl: init_acpi_config should return rc in exit path
2016-12-16 15:10 ` Boris Ostrovsky
@ 2016-12-16 15:16 ` Wei Liu
2016-12-16 15:21 ` Boris Ostrovsky
0 siblings, 1 reply; 12+ messages in thread
From: Wei Liu @ 2016-12-16 15:16 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: Andrew Cooper, Wei Liu, Ian Jackson, Xen-devel
On Fri, Dec 16, 2016 at 10:10:43AM -0500, Boris Ostrovsky wrote:
> On 12/14/2016 06:49 AM, Andrew Cooper wrote:
> > On 14/12/16 11:44, Wei Liu wrote:
> >> ... otherwise it returns 0 even if the function fails.
> >
> > Coverity-ID: 1397121
> >
> >> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> >
> > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >
> >> ---
> >> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> >>
> >> This should be backported to 4.8.
> >> ---
> >> tools/libxl/libxl_x86_acpi.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/tools/libxl/libxl_x86_acpi.c b/tools/libxl/libxl_x86_acpi.c
> >> index 686ac8e..6cf0c30 100644
> >> --- a/tools/libxl/libxl_x86_acpi.c
> >> +++ b/tools/libxl/libxl_x86_acpi.c
> >> @@ -154,7 +154,7 @@ static int init_acpi_config(libxl__gc *gc,
> >> config->acpi_revision = 5;
> >>
> >> out:
> >> - return 0;
> >> + return rc;
>
> This breaks init_acpi_config(): xc_domain_getinfo() at the top of
> returns a positive number (number of domains).
>
I see, I think we also need to set rc=0 in the success path.
Wei.
> -boris
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] libxl: init_acpi_config should return rc in exit path
2016-12-16 15:16 ` Wei Liu
@ 2016-12-16 15:21 ` Boris Ostrovsky
2016-12-16 15:20 ` Wei Liu
0 siblings, 1 reply; 12+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 15:21 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, Ian Jackson, Xen-devel
On 12/16/2016 10:16 AM, Wei Liu wrote:
>
> I see, I think we also need to set rc=0 in the success path.
Or 'return rc < 0 ? rc : 0;'
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] libxl: init_acpi_config should return rc in exit path
2016-12-16 15:21 ` Boris Ostrovsky
@ 2016-12-16 15:20 ` Wei Liu
2016-12-16 15:42 ` Ian Jackson
0 siblings, 1 reply; 12+ messages in thread
From: Wei Liu @ 2016-12-16 15:20 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: Andrew Cooper, Wei Liu, Ian Jackson, Xen-devel
On Fri, Dec 16, 2016 at 10:21:07AM -0500, Boris Ostrovsky wrote:
> On 12/16/2016 10:16 AM, Wei Liu wrote:
> >
> > I see, I think we also need to set rc=0 in the success path.
>
> Or 'return rc < 0 ? rc : 0;'
>
I don't think so, because there could be functions that only return >0
values to indicate error.
Wei.
> -boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] libxl: init_acpi_config should return rc in exit path
2016-12-16 15:20 ` Wei Liu
@ 2016-12-16 15:42 ` Ian Jackson
0 siblings, 0 replies; 12+ messages in thread
From: Ian Jackson @ 2016-12-16 15:42 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, Boris Ostrovsky, Ian Jackson, Xen-devel
Wei Liu writes ("Re: [Xen-devel] [PATCH] libxl: init_acpi_config should return rc in exit path"):
> I don't think so, because there could be functions that only return >0
> values to indicate error.
The problem here is the violation of the CODING_STYLE which forbids
the use of `rc' to contain anything other than a libxl error value.
`r' should be used for the values from libxc. And each failure path
should set rc.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] libxl: init_acpi_config should return rc in exit path
2016-12-14 11:44 [PATCH] libxl: init_acpi_config should return rc in exit path Wei Liu
2016-12-14 11:49 ` Andrew Cooper
@ 2016-12-16 11:05 ` Ian Jackson
2016-12-16 11:09 ` Wei Liu
2016-12-16 16:23 ` Wei Liu
2 siblings, 1 reply; 12+ messages in thread
From: Ian Jackson @ 2016-12-16 11:05 UTC (permalink / raw)
To: Wei Liu; +Cc: Xen-devel
Wei Liu writes ("[PATCH] libxl: init_acpi_config should return rc in exit path"):
> ... otherwise it returns 0 even if the function fails.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
FTR, I'm happy for obvious patches from you, like this one, to go in
without waiting for my ack!
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] libxl: init_acpi_config should return rc in exit path
2016-12-16 11:05 ` Ian Jackson
@ 2016-12-16 11:09 ` Wei Liu
0 siblings, 0 replies; 12+ messages in thread
From: Wei Liu @ 2016-12-16 11:09 UTC (permalink / raw)
To: Ian Jackson; +Cc: Xen-devel, Wei Liu
On Fri, Dec 16, 2016 at 11:05:03AM +0000, Ian Jackson wrote:
> Wei Liu writes ("[PATCH] libxl: init_acpi_config should return rc in exit path"):
> > ... otherwise it returns 0 even if the function fails.
>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>
Thanks.
> FTR, I'm happy for obvious patches from you, like this one, to go in
> without waiting for my ack!
>
Noted.
> Thanks,
> Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] libxl: init_acpi_config should return rc in exit path
2016-12-14 11:44 [PATCH] libxl: init_acpi_config should return rc in exit path Wei Liu
2016-12-14 11:49 ` Andrew Cooper
2016-12-16 11:05 ` Ian Jackson
@ 2016-12-16 16:23 ` Wei Liu
2016-12-16 16:25 ` Ian Jackson
2 siblings, 1 reply; 12+ messages in thread
From: Wei Liu @ 2016-12-16 16:23 UTC (permalink / raw)
To: Xen-devel; +Cc: Ian Jackson, Wei Liu
On Wed, Dec 14, 2016 at 11:44:36AM +0000, Wei Liu wrote:
> ... otherwise it returns 0 even if the function fails.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>
> This should be backported to 4.8.
And please don't just apply this one patch because it is buggy.
You would also need "libxl: set rc to 0 in init_acpi_config in success
path".
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-12-16 16:25 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-14 11:44 [PATCH] libxl: init_acpi_config should return rc in exit path Wei Liu
2016-12-14 11:49 ` Andrew Cooper
2016-12-14 12:28 ` Wei Liu
2016-12-16 15:10 ` Boris Ostrovsky
2016-12-16 15:16 ` Wei Liu
2016-12-16 15:21 ` Boris Ostrovsky
2016-12-16 15:20 ` Wei Liu
2016-12-16 15:42 ` Ian Jackson
2016-12-16 11:05 ` Ian Jackson
2016-12-16 11:09 ` Wei Liu
2016-12-16 16:23 ` Wei Liu
2016-12-16 16:25 ` Ian Jackson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.