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

Hi,

Since commit efc6c07 ("configure: Add a test for the minimum compiler version=
"),
QEMU explicitely depends on GCC >=3D 4.8.

Marc-Andr=C3=A9 Lureau (2):
  Remove GCC version checks (all < 4.8)
  compiler.h: remove QEMU_GNUC_PREREQ macro

 include/qemu/atomic.h         | 17 -----------------
 include/qemu/compiler.h       | 33 ++++++---------------------------
 include/qemu/qemu-plugin.h    |  9 ++-------
 scripts/cocci-macro-file.h    |  1 -
 tools/virtiofsd/fuse_common.h |  4 +---
 accel/tcg/cpu-exec.c          |  2 +-
 tests/tcg/arm/fcvt.c          |  8 +++-----
 7 files changed, 13 insertions(+), 61 deletions(-)

--=20
2.29.0

Patchew URL: https://patchew.org/QEMU/20201124125235.266884-1-marcandre.lureau@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20201124125235.266884-1-marcandre.lureau@redhat.com
Subject: [PATCH 0/2] Remove GCC < 4.8 checks

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20201124122936.30588-1-kraxel@redhat.com -> patchew/20201124122936.30588-1-kraxel@redhat.com
 * [new tag]         patchew/20201124125235.266884-1-marcandre.lureau@redhat.com -> patchew/20201124125235.266884-1-marcandre.lureau@redhat.com
Switched to a new branch 'test'
c9fd76f compiler.h: remove QEMU_GNUC_PREREQ macro
4428f55 Remove GCC version checks (all < 4.8)

=== OUTPUT BEGIN ===
1/2 Checking commit 4428f552871c (Remove GCC version checks (all < 4.8))
WARNING: architecture specific defines should be avoided
#22: FILE: accel/tcg/cpu-exec.c:727:
+#if defined(__clang__)

WARNING: Block comments use a leading /* on a separate line
#86: FILE: include/qemu/compiler.h:105:
+   /* Map __printf__ to __gnu_printf__ because we want standard format strings

WARNING: Block comments use a trailing */ on a separate line
#87: FILE: include/qemu/compiler.h:106:
+    * even when MinGW or GLib include files use __printf__. */

ERROR: space prohibited between function name and open parenthesis '('
#129: FILE: tests/tcg/arm/fcvt.c:76:
+# define SNANF (__builtin_nansf (""))

ERROR: space prohibited between function name and open parenthesis '('
#130: FILE: tests/tcg/arm/fcvt.c:77:
+# define SNAN (__builtin_nans (""))

ERROR: space prohibited between function name and open parenthesis '('
#131: FILE: tests/tcg/arm/fcvt.c:78:
+# define SNANL (__builtin_nansl (""))

WARNING: architecture specific defines should be avoided
#146: FILE: tools/virtiofsd/fuse_common.h:813:
+#if defined(__GNUC__) && !defined __cplusplus

total: 3 errors, 4 warnings, 106 lines checked

Patch 1/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/2 Checking commit c9fd76f82866 (compiler.h: remove QEMU_GNUC_PREREQ macro)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20201124125235.266884-1-marcandre.lureau@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

On Tue, 24 Nov 2020 at 12:52, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi,
>
> Since commit efc6c07 ("configure: Add a test for the minimum compiler version=
> "),
> QEMU explicitely depends on GCC >= 4.8.

You need to be a bit cautious about removing QEMU_GNUC_PREREQ()
checks, because clang advertises itself as GCC 4.2 via the
__GNUC__ and __GNUC_MINOR__ macros that QEMU_GNUC_PREREQ()
tests, and so unless the code has explicitly handled clang
via a different ifdef or feature test clang will be using
the fallback codepath in cases like this. So you also need
to find out whether all our supported versions of clang
are OK with the gcc-4.4-and-up version of the code before
removing any particular ifdef.

Compare this previous (not-applied) patchset from Philippe:
 https://patchew.org/QEMU/20200928125859.734287-1-philmd@redhat.com/
which dealt with two of these ifdefs, one of which is
"clearly OK" and the other of which is "needs more analysis".
The path forward I think is along those lines -- we need
one patch per ifdef we're trying to remove, and the commit
message can then include the information about why in
this case it is OK for clang too (or switch the ifdef
to check for something else, eg one of clang's feature-specific
test macros).

thanks
-- PMM

On Tue, Nov 24, 2020 at 5:33 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Tue, 24 Nov 2020 at 12:52, <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Hi,
> >
> > Since commit efc6c07 ("configure: Add a test for the minimum compiler
> version=
> > "),
> > QEMU explicitely depends on GCC >= 4.8.
>
> You need to be a bit cautious about removing QEMU_GNUC_PREREQ()
> checks, because clang advertises itself as GCC 4.2 via the
> __GNUC__ and __GNUC_MINOR__ macros that QEMU_GNUC_PREREQ()
> tests, and so unless the code has explicitly handled clang
> via a different ifdef or feature test clang will be using
> the fallback codepath in cases like this. So you also need
> to find out whether all our supported versions of clang
> are OK with the gcc-4.4-and-up version of the code before
> removing any particular ifdef.
>
> Compare this previous (not-applied) patchset from Philippe:
>  https://patchew.org/QEMU/20200928125859.734287-1-philmd@redhat.com/
> which dealt with two of these ifdefs, one of which is
> "clearly OK" and the other of which is "needs more analysis".
> The path forward I think is along those lines -- we need
> one patch per ifdef we're trying to remove, and the commit
> message can then include the information about why in
> this case it is OK for clang too (or switch the ifdef
> to check for something else, eg one of clang's feature-specific
> test macros).
>


Thanks for pointing out the series, I missed it. Ok, I'll try to do some
more research.

-- 
Marc-André Lureau

On Tue, Nov 24, 2020 at 4:54 PM <marcandre.lureau@redhat.com> wrote:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi,
>
> Since commit efc6c07 ("configure: Add a test for the minimum compiler
> version=
> "),
> QEMU explicitely depends on GCC >=3D 4.8.
>
> Marc-Andr=C3=A9 Lureau (2):
>   Remove GCC version checks (all < 4.8)
>   compiler.h: remove QEMU_GNUC_PREREQ macro
>

There is some double-encoding going on with my cover letters, lovely...


>  include/qemu/atomic.h         | 17 -----------------
>  include/qemu/compiler.h       | 33 ++++++---------------------------
>  include/qemu/qemu-plugin.h    |  9 ++-------
>  scripts/cocci-macro-file.h    |  1 -
>  tools/virtiofsd/fuse_common.h |  4 +---
>  accel/tcg/cpu-exec.c          |  2 +-
>  tests/tcg/arm/fcvt.c          |  8 +++-----
>  7 files changed, 13 insertions(+), 61 deletions(-)
>
> --=20
> 2.29.0
>
>
>
>

-- 
Marc-André Lureau