| Message ID | 20191128022458.4428-1-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools/arm: include xen-tools/libs.h from libxl_arm_acpi.c | expand |
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 >
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 >>
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.
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.
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;
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(+)