Re: [Qemu-devel] [PATCH v7 11/11] disas: Add capstone as submodule

[Philippe Mathieu-Daudé]

Hi Richard,

>> On 10/21/2017 09:46 PM, Richard Henderson wrote:
>>> Do not require the submodule, but use it if present.  Allow the
>>> command-line to override system or git submodule either way.
>>>
>>> Signed-off-by: Richard Henderson <address@hidden>
>>> ---
>>>  Makefile    | 13 +++++++++++++
>>>  .gitmodules |  3 +++
>>>  capstone    |  1 +
>>>  configure   | 60 
>>> +++++++++++++++++++++++++++++++++++++++++++++++++-----------
>>>  4 files changed, 66 insertions(+), 11 deletions(-)
>>>  create mode 160000 capstone
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 9372742f86..beecc85bee 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -383,6 +383,19 @@ subdir-dtc: .git-submodule-status dtc/libfdt dtc/tests
>>>  dtc/%: .git-submodule-status
>>>     mkdir -p $@
>>>  
>>> +# Overriding CFLAGS causes us to lose defines added in the sub-makefile.
>>> +# Not overriding CFLAGS leads to mis-matches between compilation modes.
>>> +# Therefore we replicate some of the logic in the sub-makefile.
>>
>> I'm having plenty of "missing-prototypes" warnings:
> 
> Yes, we use lots of -Wfoo that upstream Capstone does not.  I do strip 
> -Werror,
> so at least it builds.  Are you suggesting that I drop most of our extra 
> -Wfoo?

Exactly. It's unlikely we try to modify capstone code in the submodule
to silent the warnings, there is enough QEMU work to do :P
I'd personally go with:

CAP_CFLAGS = $(CFLAGS) $(QEMU_CFLAGS) -w

At worst if there is an error while building capstone, it will still get
displayed.

> I suppose that's reasonable.  We don't want our developers worrying about
> warnings coming from upstream code.  If in fact you believe that most of our
> developers won't just install libcapstone-dev and be done?

This was my first reflex, but then you added the git feature and I just
wanted to test it.

Usually if I don't need a cutting edge feature, I try to use distrib
packages, to keep my environment closer to other developers.

A big part of QEMU developers uses Redhat/Fedora or Debian/Ubuntu and
there is an effort to verify QEMU still builds with those distribs using
the Travis CI. Now there even are VMs for BSD folks :)

So I'd not worry about distrib packages and these upstream warnings.

Regards,

Phil.