[Xen-devel] [PATCH] tools/arm: include xen-tools/libs.h from libxl_arm_acpi.c

Stefano Stabellini posted 1 patch 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20191128022458.4428-1-sstabellini@kernel.org
tools/libxl/libxl_arm_acpi.c | 1 +
1 file changed, 1 insertion(+)

[Xen-devel] [PATCH] tools/arm: include xen-tools/libs.h from libxl_arm_acpi.c

Posted by Stefano Stabellini 1 week ago
libxl_arm_acpi.c is using BUILD_BUG_ON but it is not including
xen-tools/libs.h that defines it.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 tools/libxl/libxl_arm_acpi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index ba874c3d32..52c476ff65 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -19,6 +19,7 @@
 #include "libxl_arm.h"
 
 #include <stdint.h>
+#include <xen-tools/libs.h>
 
 /* Below typedefs are useful for the headers under acpi/ */
 typedef uint8_t u8;
-- 
2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/arm: include xen-tools/libs.h from libxl_arm_acpi.c

Posted by Wei Liu 1 week ago
On Wed, Nov 27, 2019 at 06:24:58PM -0800, Stefano Stabellini wrote:
> libxl_arm_acpi.c is using BUILD_BUG_ON but it is not including
> xen-tools/libs.h that defines it.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>

Acked-by: Wei Liu <wl@xen.org>

Juergen, this is a trivial patch. I think it can go in 4.13.

Wei.

> ---
>  tools/libxl/libxl_arm_acpi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index ba874c3d32..52c476ff65 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -19,6 +19,7 @@
>  #include "libxl_arm.h"
>  
>  #include <stdint.h>
> +#include <xen-tools/libs.h>
>  
>  /* Below typedefs are useful for the headers under acpi/ */
>  typedef uint8_t u8;
> -- 
> 2.17.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/arm: include xen-tools/libs.h from libxl_arm_acpi.c

Posted by Jürgen Groß 1 week ago
On 28.11.19 11:15, Wei Liu wrote:
> On Wed, Nov 27, 2019 at 06:24:58PM -0800, Stefano Stabellini wrote:
>> libxl_arm_acpi.c is using BUILD_BUG_ON but it is not including
>> xen-tools/libs.h that defines it.
>>
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> Acked-by: Wei Liu <wl@xen.org>
> 
> Juergen, this is a trivial patch. I think it can go in 4.13.

Why is this patch needed?

tools/libxl/libxl_arm_acpi.c includes libxl_arm.h, which includes
libxl_internal.h, which includes xen-tools/libs.h.

So this is a purely cosmetic patch, especially as libxl_arm.h and
libxl_internal.h are libxl-internal includes, so there is a very low
risk for the include of xen-tools/libs.h suddenly disappearing, and
even it would due to a patch of one of those include files, it would
be detected by a failing build immediately.

So I won't take it for 4.13, even if being trivial.


Juergen

> 
> Wei.
> 
>> ---
>>   tools/libxl/libxl_arm_acpi.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
>> index ba874c3d32..52c476ff65 100644
>> --- a/tools/libxl/libxl_arm_acpi.c
>> +++ b/tools/libxl/libxl_arm_acpi.c
>> @@ -19,6 +19,7 @@
>>   #include "libxl_arm.h"
>>   
>>   #include <stdint.h>
>> +#include <xen-tools/libs.h>
>>   
>>   /* Below typedefs are useful for the headers under acpi/ */
>>   typedef uint8_t u8;
>> -- 
>> 2.17.1
>>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/arm: include xen-tools/libs.h from libxl_arm_acpi.c

Posted by Wei Liu 1 week ago
On Thu, Nov 28, 2019 at 11:30:34AM +0100, Jürgen Groß wrote:
> On 28.11.19 11:15, Wei Liu wrote:
> > On Wed, Nov 27, 2019 at 06:24:58PM -0800, Stefano Stabellini wrote:
> > > libxl_arm_acpi.c is using BUILD_BUG_ON but it is not including
> > > xen-tools/libs.h that defines it.
> > > 
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > 
> > Acked-by: Wei Liu <wl@xen.org>
> > 
> > Juergen, this is a trivial patch. I think it can go in 4.13.
> 
> Why is this patch needed?
> 
> tools/libxl/libxl_arm_acpi.c includes libxl_arm.h, which includes
> libxl_internal.h, which includes xen-tools/libs.h.

Oh I missed that.

In that case I don't think this patch is required for 4.13.

Stefano, did you see a build error or something?

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/arm: include xen-tools/libs.h from libxl_arm_acpi.c

Posted by Stefano Stabellini 1 week ago
On Thu, 28 Nov 2019, Wei Liu wrote:
> On Thu, Nov 28, 2019 at 11:30:34AM +0100, Jürgen Groß wrote:
> > On 28.11.19 11:15, Wei Liu wrote:
> > > On Wed, Nov 27, 2019 at 06:24:58PM -0800, Stefano Stabellini wrote:
> > > > libxl_arm_acpi.c is using BUILD_BUG_ON but it is not including
> > > > xen-tools/libs.h that defines it.
> > > > 
> > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > > 
> > > Acked-by: Wei Liu <wl@xen.org>
> > > 
> > > Juergen, this is a trivial patch. I think it can go in 4.13.
> > 
> > Why is this patch needed?
> > 
> > tools/libxl/libxl_arm_acpi.c includes libxl_arm.h, which includes
> > libxl_internal.h, which includes xen-tools/libs.h.
> 
> Oh I missed that.
> 
> In that case I don't think this patch is required for 4.13.
> 
> Stefano, did you see a build error or something?
> 

Hi Wei, and Jurgen,

Thanks for the review and also for probably catching a mistake in the
patch.

Yes, this patch fixes a build error with the latest Yocto and gcc 9:

| libxl_arm_acpi.c: In function 'make_acpi_rsdp':
| libxl_arm_acpi.c:193:5: error: implicit declaration of function 'BUILD_BUG_ON' [-Werror=implicit-function-declaration]
|   193 |     BUILD_BUG_ON(sizeof(ACPI_OEM_ID) != sizeof(rsdp->oem_id));
|       |     ^~~~~~~~~~~~

but the error was based on an older Xen tree based on 4.11, which
doesn't have an include of xen-tools/libs.h in libxl_internal.h. So, I
think Juergen is right that this should not be needed upstream.

I didn't actually have a repro (the issue was reported to me by somebody
else), so before I sent the patch to xen-devel I manually check the code
but couldn't actually try a build. And I didn't notice the include
xen-tools/libs.h in libxl_internal.h. Sorry about that._______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel