diff mbox

[[PATCH] ] libxl/arm: Fix ARM build.

Message ID 1494930198-3804-1-git-send-email-andrii.anisov@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrii Anisov May 16, 2017, 10:23 a.m. UTC 
From: Andrii Anisov <andrii_anisov@epam.com>

Some compilers do not (validly?) detect that size will always be
initialized when (rc > 0), so implicitly initialize it.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 tools/libxl/libxl_arm_acpi.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Andrii Anisov May 16, 2017, 10:59 a.m. UTC | #1 
CC Ian Jakson and Wei Liu as maintainers.

Sincerely,
Andrii Anisov.

2017-05-16 13:23 GMT+03:00 Andrii Anisov <andrii.anisov@gmail.com>:

> From: Andrii Anisov <andrii_anisov@epam.com>
>
> Some compilers do not (validly?) detect that size will always be
> initialized when (rc > 0), so implicitly initialize it.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  tools/libxl/libxl_arm_acpi.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index db113db..f61aec6 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -73,6 +73,8 @@ static int libxl__estimate_madt_size(libxl__gc *gc,
>  {
>      int rc = 0;
>
> +    *size = 0;
> +
>      switch (info->arch_arm.gic_version) {
>      case LIBXL_GIC_VERSION_V2:
>          *size = sizeof(struct acpi_table_madt) +
> --
> 2.7.4
>
>
Wei Liu May 16, 2017, 11:03 a.m. UTC | #2 
On Tue, May 16, 2017 at 01:59:40PM +0300, Andrii Anisov wrote:
> CC Ian Jakson and Wei Liu as maintainers.
> 
> Sincerely,
> Andrii Anisov.
> 
> 2017-05-16 13:23 GMT+03:00 Andrii Anisov <andrii.anisov@gmail.com>:
> 
> > From: Andrii Anisov <andrii_anisov@epam.com>
> >
> > Some compilers do not (validly?) detect that size will always be
> > initialized when (rc > 0), so implicitly initialize it.
> >

I'm confused by the commit message.

When rc > 0 (the default branch), size is not initialised. I guess
that's what gcc complains about?

If I'm right, would it be better to set *size to 0 in the default
branch?

> > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > ---
> >  tools/libxl/libxl_arm_acpi.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> > index db113db..f61aec6 100644
> > --- a/tools/libxl/libxl_arm_acpi.c
> > +++ b/tools/libxl/libxl_arm_acpi.c
> > @@ -73,6 +73,8 @@ static int libxl__estimate_madt_size(libxl__gc *gc,
> >  {
> >      int rc = 0;
> >
> > +    *size = 0;
> > +
> >      switch (info->arch_arm.gic_version) {
> >      case LIBXL_GIC_VERSION_V2:
> >          *size = sizeof(struct acpi_table_madt) +
> > --
> > 2.7.4
> >
> >
Wei Liu May 16, 2017, 11:06 a.m. UTC | #3 
On Tue, May 16, 2017 at 12:03:49PM +0100, Wei Liu wrote:
> On Tue, May 16, 2017 at 01:59:40PM +0300, Andrii Anisov wrote:
> > CC Ian Jakson and Wei Liu as maintainers.
> > 
> > Sincerely,
> > Andrii Anisov.
> > 
> > 2017-05-16 13:23 GMT+03:00 Andrii Anisov <andrii.anisov@gmail.com>:
> > 
> > > From: Andrii Anisov <andrii_anisov@epam.com>
> > >
> > > Some compilers do not (validly?) detect that size will always be
> > > initialized when (rc > 0), so implicitly initialize it.
> > >
> 
> I'm confused by the commit message.
> 
> When rc > 0 (the default branch), size is not initialised. I guess

Also rc can't be >0 because ERROR_* is negative...
Andrii Anisov May 16, 2017, 3:14 p.m. UTC | #4 
Dear Wei,


On 16.05.17 14:03, Wei Liu wrote:
> I'm confused by the commit message.

My bad, I did not pay enough attention to write the message. Would 
following be better?

     Implicitly initialize a referenced ouput parameter for all code 
branches
     in order to not face a "variable may be used uninitialized" 
compilation
     error in a caller function, fired by some compilers under 
circumstances.

> If I'm right, would it be better to set *size to 0 in the default
> branch?

Agree.
Wei Liu May 16, 2017, 3:27 p.m. UTC | #5 
On Tue, May 16, 2017 at 06:14:43PM +0300, Andrii Anisov wrote:
> Dear Wei,
> 
> 
> On 16.05.17 14:03, Wei Liu wrote:
> > I'm confused by the commit message.
> 
> My bad, I did not pay enough attention to write the message. Would following
> be better?
> 
>     Implicitly initialize a referenced ouput parameter for all code branches
>     in order to not face a "variable may be used uninitialized" compilation
>     error in a caller function, fired by some compilers under circumstances.
> 
> > If I'm right, would it be better to set *size to 0 in the default
> > branch?
> 
> Agree.
> 

I suspect if you take this approach the commit message you proposed
doesn't quite match either.

Just say:

Initialise *size in default branch to prevent certain compilers (which
ones?) from reporting "variable may be used uninitialized" errors in
caller function.


> -- 
> 
> *Andrii Anisov*
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
diff mbox

Patch

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index db113db..f61aec6 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -73,6 +73,8 @@  static int libxl__estimate_madt_size(libxl__gc *gc,
 {
     int rc = 0;
 
+    *size = 0;
+
     switch (info->arch_arm.gic_version) {
     case LIBXL_GIC_VERSION_V2:
         *size = sizeof(struct acpi_table_madt) +