Message ID | CALfaEa1Uqo-uW5y01oCoCxAHUxzDjHTEcLF505Uk0Tg47Kk-Ew@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Tue, Jun 6, 2017 at 1:20 PM, Bruno Dominguez <bru.dominguez@gmail.com> wrote: > sending again the patch with the correct format and: > > 1. Matching --extra-cxxflags description with --cxx > 2. Removing some extra spaces indicated by checkpatch script. The code looks fine to me: Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> A few more points about patch submission: All email body text above '---' is the commit description that is included in the git log. Therefore the commit description should only contain information that is useful in the version history, like the rationale for this commit. Please put changelog information below '---' so it does not become part of the git log. Once the patch has been merged it's not useful to know that you're "sending again the patch with the correct format and:" and so on. The submission guidelines page says "send each new revision as a new top-level thread, rather than burying it in-reply-to an earlier revision, as many reviewers are not looking inside deep threads for new patches". There are tools that make it easy to send and manage patch series. That way you don't have to invoke git-format-patch/git-send-email and perform manual steps. The tool I use is git-publish: https://github.com/stefanha/git-publish Putting this all together, please send a new top-level email like this: Subject: [PATCH v2] configure: split c and cxx extra flags There was no possibility to add specific cxx flags using the configure file. So A new entrance has been created to support it. Duplication of information in configure and rules.mak. Taking QEMU_CFLAGS and add them to QEMU_CXXFLAGS, now the value of QEMU_CXXFLAGS is stored in config-host.mak, so there is no need for it. The makefile for libvixl was adding flags for QEMU_CXXFLAGS in QEMU_CFLAGS because of the addition in rules.mak. That was removed, so adding them where it should be. Signed-off-by: Bruno Dominguez <bru.dominguez@gmail.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> --- v2: * Matching --extra-cxxflags description with --cxx * Removing some extra spaces indicated by checkpatch script * Fixed patch format ...
ok, thanks for the clarification. 2017-06-06 13:38 GMT+01:00 Stefan Hajnoczi <stefanha@gmail.com>: > On Tue, Jun 6, 2017 at 1:20 PM, Bruno Dominguez <bru.dominguez@gmail.com> wrote: >> sending again the patch with the correct format and: >> >> 1. Matching --extra-cxxflags description with --cxx >> 2. Removing some extra spaces indicated by checkpatch script. > > The code looks fine to me: > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > > A few more points about patch submission: > > All email body text above '---' is the commit description that is > included in the git log. Therefore the commit description should only > contain information that is useful in the version history, like the > rationale for this commit. > > Please put changelog information below '---' so it does not become > part of the git log. Once the patch has been merged it's not useful > to know that you're "sending again the patch with the correct format > and:" and so on. > > The submission guidelines page says "send each new revision as a new > top-level thread, rather than burying it in-reply-to an earlier > revision, as many reviewers are not looking inside deep threads for > new patches". > > There are tools that make it easy to send and manage patch series. > That way you don't have to invoke git-format-patch/git-send-email and > perform manual steps. The tool I use is git-publish: > https://github.com/stefanha/git-publish > > Putting this all together, please send a new top-level email like this: > > Subject: [PATCH v2] configure: split c and cxx extra flags > > There was no possibility to add specific cxx flags using the configure > file. So A new entrance has been created to support it. > > Duplication of information in configure and rules.mak. Taking > QEMU_CFLAGS and add them to QEMU_CXXFLAGS, now the value of > QEMU_CXXFLAGS is stored in config-host.mak, so there is no need for > it. > > The makefile for libvixl was adding flags for QEMU_CXXFLAGS in > QEMU_CFLAGS because of the addition in rules.mak. That was removed, so > adding them where it should be. > > Signed-off-by: Bruno Dominguez <bru.dominguez@gmail.com> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > v2: > * Matching --extra-cxxflags description with --cxx > * Removing some extra spaces indicated by checkpatch script > * Fixed patch format > > ...
diff --git a/configure b/configure index 0586ec9..4ab0d0f 100755 --- a/configure +++ b/configure @@ -91,7 +91,8 @@ update_cxxflags() { # Set QEMU_CXXFLAGS from QEMU_CFLAGS by filtering out those # options which some versions of GCC's C++ compiler complain about # because they only make sense for C programs. - QEMU_CXXFLAGS= + QEMU_CXXFLAGS="$QEMU_CXXFLAGS -D__STDC_LIMIT_MACROS" + for arg in $QEMU_CFLAGS; do case $arg in -Wstrict-prototypes|-Wmissing-prototypes|-Wnested-externs|\ @@ -344,6 +345,9 @@ for opt do --extra-cflags=*) QEMU_CFLAGS="$QEMU_CFLAGS $optarg" EXTRA_CFLAGS="$optarg" ;; + --extra-cxxflags=*) QEMU_CXXFLAGS="$QEMU_CXXFLAGS $optarg" + EXTRA_CXXFLAGS="$optarg" + ;; --extra-ldflags=*) LDFLAGS="$LDFLAGS $optarg" EXTRA_LDFLAGS="$optarg" ;; @@ -787,6 +791,8 @@ for opt do ;; --extra-cflags=*) ;; + --extra-cxxflags=*) + ;; --extra-ldflags=*) ;; --enable-debug-info) @@ -1304,6 +1310,7 @@ Advanced options (experts only): --cxx=CXX use C++ compiler CXX [$cxx] --objcc=OBJCC use Objective-C compiler OBJCC [$objcc] --extra-cflags=CFLAGS append extra C compiler flags QEMU_CFLAGS + --extra-cxxflags=CXXFLAGS append extra C++ compiler flags QEMU_CXXFLAGS --extra-ldflags=LDFLAGS append extra linker flags LDFLAGS --make=MAKE use specified make [$make] --install=INSTALL use specified install [$install] @@ -1489,37 +1496,6 @@ if test "$bogus_os" = "yes"; then error_exit "Unrecognized host OS $targetos" fi -# Check that the C++ compiler exists and works with the C compiler -if has $cxx; then - cat > $TMPC <<EOF -int c_function(void); -int main(void) { return c_function(); } -EOF - - compile_object - - cat > $TMPCXX <<EOF -extern "C" { - int c_function(void); -} -int c_function(void) { return 42; } -EOF - - update_cxxflags - - if do_cxx $QEMU_CXXFLAGS -o $TMPE $TMPCXX $TMPO $LDFLAGS; then - # C++ compiler $cxx works ok with C compiler $cc - : - else - echo "C++ compiler $cxx does not work with C compiler $cc" - echo "Disabling C++ specific optional code" - cxx= - fi -else - echo "No C++ compiler available; disabling C++ specific optional code" - cxx= -fi - gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits" gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags" gcc_flags="-Wno-missing-include-dirs -Wempty-body -Wnested-externs $gcc_flags" @@ -5063,6 +5039,38 @@ EOF fi fi +# Check that the C++ compiler exists and works with the C compiler. +# All the QEMU_CXXFLAGS are based on QEMU_CFLAGS. Keep this at the end to don't miss any other that could be added. +if has $cxx; then + cat > $TMPC <<EOF +int c_function(void); +int main(void) { return c_function(); } +EOF + + compile_object + + cat > $TMPCXX <<EOF +extern "C" { + int c_function(void); +} +int c_function(void) { return 42; } +EOF + + update_cxxflags + + if do_cxx $QEMU_CXXFLAGS -o $TMPE $TMPCXX $TMPO $LDFLAGS; then + # C++ compiler $cxx works ok with C compiler $cc + : + else + echo "C++ compiler $cxx does not work with C compiler $cc" + echo "Disabling C++ specific optional code" + cxx= + fi +else + echo "No C++ compiler available; disabling C++ specific optional code" + cxx= +fi + echo_version() { if test "$1" = "yes" ; then echo "($2)" @@ -5268,6 +5276,7 @@ if test "$mingw32" = "no" ; then fi echo "qemu_helperdir=$libexecdir" >> $config_host_mak echo "extra_cflags=$EXTRA_CFLAGS" >> $config_host_mak +echo "extra_cxxflags=$EXTRA_CXXFLAGS" >> $config_host_mak echo "extra_ldflags=$EXTRA_LDFLAGS" >> $config_host_mak echo "qemu_localedir=$qemu_localedir" >> $config_host_mak echo "libs_softmmu=$libs_softmmu" >> $config_host_mak @@ -5906,6 +5915,7 @@ echo "WINDRES=$windres" >> $config_host_mak echo "CFLAGS=$CFLAGS" >> $config_host_mak echo "CFLAGS_NOPIE=$CFLAGS_NOPIE" >> $config_host_mak echo "QEMU_CFLAGS=$QEMU_CFLAGS" >> $config_host_mak +echo "QEMU_CXXFLAGS=$QEMU_CXXFLAGS" >> $config_host_mak echo "QEMU_INCLUDES=$QEMU_INCLUDES" >> $config_host_mak if test "$sparse" = "yes" ; then echo "CC := REAL_CC=\"\$(CC)\" cgcc" >> $config_host_mak diff --git a/disas/libvixl/Makefile.objs b/disas/libvixl/Makefile.objs index bbe7695..dbf7def 100644 --- a/disas/libvixl/Makefile.objs +++ b/disas/libvixl/Makefile.objs @@ -6,6 +6,6 @@ libvixl_OBJS = vixl/utils.o \ # The -Wno-sign-compare is needed only for gcc 4.6, which complains about # some signed-unsigned equality comparisons which later gcc versions do not. -$(addprefix $(obj)/,$(libvixl_OBJS)): QEMU_CFLAGS := -I$(SRC_PATH)/disas/libvixl $(QEMU_CFLAGS) -Wno-sign-compare +$(addprefix $(obj)/,$(libvixl_OBJS)): QEMU_CXXFLAGS := -I$(SRC_PATH)/disas/libvixl $(QEMU_CXXFLAGS) -Wno-sign-compare common-obj-$(CONFIG_ARM_A64_DIS) += $(libvixl_OBJS) diff --git a/rules.mak b/rules.mak index 1c0eabb..2a2fb72 100644 --- a/rules.mak +++ b/rules.mak @@ -20,9 +20,6 @@ MAKEFLAGS += -rR %.mak: clean-target: -# Flags for C++ compilation -QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out -Wstrict-prototypes -Wmissing-prototypes -Wnested-externs -Wold-style-declaration -Wold-style-definition -Wredundant-decls, $(QEMU_CFLAGS)) - # Flags for dependency generation QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
sending again the patch with the correct format and: 1. Matching --extra-cxxflags description with --cxx 2. Removing some extra spaces indicated by checkpatch script. Signed-off-by: Bruno Dominguez <bru.dominguez@gmail.com> --- configure | 74 +++++++++++++++++++++++++-------------------- disas/libvixl/Makefile.objs | 2 +- rules.mak | 3 -- 3 files changed, 43 insertions(+), 36 deletions(-) -- 1.9.1