diff mbox

split c and cxx extra flags

Message ID CALfaEa1Uqo-uW5y01oCoCxAHUxzDjHTEcLF505Uk0Tg47Kk-Ew@mail.gmail.com
State New
Headers show

Commit Message

Bruno Dominguez June 6, 2017, 12:20 p.m. UTC
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

Comments

Stefan Hajnoczi June 6, 2017, 12:38 p.m. UTC | #1
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

...
Bruno Dominguez June 6, 2017, 12:59 p.m. UTC | #2
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 mbox

Patch

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