[Qemu-devel] [PATCH for-4.1] contrib/elf2dmp: Build download.o with CURL_CFLAGS

Peter Maydell posted 1 patch 6 years, 4 months ago
Test checkpatch passed
Test s390x passed
Test asan passed
Test docker-mingw@fedora passed
Test FreeBSD passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190719100955.17180-1-peter.maydell@linaro.org
Maintainers: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
Makefile                      | 1 -
contrib/elf2dmp/Makefile.objs | 3 +++
2 files changed, 3 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH for-4.1] contrib/elf2dmp: Build download.o with CURL_CFLAGS
Posted by Peter Maydell 6 years, 4 months ago
contrib/elf2dmp has a source file which uses curl/curl.h;
although we link the final executable with CURL_LIBS, we
forgot to build this source file with CURL_CFLAGS, so if
the curl header is in a place that's not already on the
system include path then it will fail to build.

Add a line specifying the cflags needed for download.o;
while we are here, bring the specification of the libs
into line with this, since using a per-object variable
setting is preferred over adding them to the final
executable link line.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I'm pretty sure this is what's causing the compile
failure described at:
https://stackoverflow.com/questions/57102476/qemu-recipe-for-target-contrib-elf2dmp-download-o-failed
I haven't actually got a setup that reproduces the error,
though, so this is tested by looking at the command lines
run on an Ubuntu setup that compiles even without the fix.

There's an argument for splitting this into two patches,
I suppose, one which just fixes the CURL_CFLAGS bug and
one which tidies the CURL_LIBS handling. But it didn't
seem worth it to me. Let me know if you'd prefer it split.
---
 Makefile                      | 1 -
 contrib/elf2dmp/Makefile.objs | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index f9791dcb827..27dabb9b1a0 100644
--- a/Makefile
+++ b/Makefile
@@ -626,7 +626,6 @@ ifneq ($(EXESUF),)
 qemu-ga: qemu-ga$(EXESUF) $(QGA_VSS_PROVIDER) $(QEMU_GA_MSI)
 endif
 
-elf2dmp$(EXESUF): LIBS += $(CURL_LIBS)
 elf2dmp$(EXESUF): $(elf2dmp-obj-y)
 	$(call LINK, $^)
 
diff --git a/contrib/elf2dmp/Makefile.objs b/contrib/elf2dmp/Makefile.objs
index e3140f58cf7..15057169160 100644
--- a/contrib/elf2dmp/Makefile.objs
+++ b/contrib/elf2dmp/Makefile.objs
@@ -1 +1,4 @@
 elf2dmp-obj-y = main.o addrspace.o download.o pdb.o qemu_elf.o
+
+download.o-cflags := $(CURL_CFLAGS)
+download.o-libs   := $(CURL_LIBS)
-- 
2.20.1


Re: [Qemu-devel] [PATCH for-4.1] contrib/elf2dmp: Build download.o with CURL_CFLAGS
Posted by Marc-André Lureau 6 years, 4 months ago
Hi

On Fri, Jul 19, 2019 at 2:10 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> contrib/elf2dmp has a source file which uses curl/curl.h;
> although we link the final executable with CURL_LIBS, we
> forgot to build this source file with CURL_CFLAGS, so if
> the curl header is in a place that's not already on the
> system include path then it will fail to build.
>
> Add a line specifying the cflags needed for download.o;
> while we are here, bring the specification of the libs
> into line with this, since using a per-object variable
> setting is preferred over adding them to the final
> executable link line.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
> I'm pretty sure this is what's causing the compile
> failure described at:
> https://stackoverflow.com/questions/57102476/qemu-recipe-for-target-contrib-elf2dmp-download-o-failed
> I haven't actually got a setup that reproduces the error,
> though, so this is tested by looking at the command lines
> run on an Ubuntu setup that compiles even without the fix.
>
> There's an argument for splitting this into two patches,
> I suppose, one which just fixes the CURL_CFLAGS bug and
> one which tidies the CURL_LIBS handling. But it didn't
> seem worth it to me. Let me know if you'd prefer it split.
> ---
>  Makefile                      | 1 -
>  contrib/elf2dmp/Makefile.objs | 3 +++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index f9791dcb827..27dabb9b1a0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -626,7 +626,6 @@ ifneq ($(EXESUF),)
>  qemu-ga: qemu-ga$(EXESUF) $(QGA_VSS_PROVIDER) $(QEMU_GA_MSI)
>  endif
>
> -elf2dmp$(EXESUF): LIBS += $(CURL_LIBS)
>  elf2dmp$(EXESUF): $(elf2dmp-obj-y)
>         $(call LINK, $^)
>
> diff --git a/contrib/elf2dmp/Makefile.objs b/contrib/elf2dmp/Makefile.objs
> index e3140f58cf7..15057169160 100644
> --- a/contrib/elf2dmp/Makefile.objs
> +++ b/contrib/elf2dmp/Makefile.objs
> @@ -1 +1,4 @@
>  elf2dmp-obj-y = main.o addrspace.o download.o pdb.o qemu_elf.o
> +
> +download.o-cflags := $(CURL_CFLAGS)
> +download.o-libs   := $(CURL_LIBS)
> --
> 2.20.1
>
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH for-4.1] contrib/elf2dmp: Build download.o with CURL_CFLAGS
Posted by Peter Maydell 6 years, 4 months ago
On Fri, 19 Jul 2019 at 11:18, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Fri, Jul 19, 2019 at 2:10 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > contrib/elf2dmp has a source file which uses curl/curl.h;
> > although we link the final executable with CURL_LIBS, we
> > forgot to build this source file with CURL_CFLAGS, so if
> > the curl header is in a place that's not already on the
> > system include path then it will fail to build.
> >
> > Add a line specifying the cflags needed for download.o;
> > while we are here, bring the specification of the libs
> > into line with this, since using a per-object variable
> > setting is preferred over adding them to the final
> > executable link line.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks. Since we don't have an obvious route for elf2dmp patches
to go into master I'll just put this in via the arm pullreq
I'm putting together for rc2.

-- PMM