include/qemu/osdep.h | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
Coverity's parser chokes on __builtin_choose_expr inside a constant
expression. Since it is used only to raise compilation errors for
non-constant arguments, we can just assume there are no such errors
in the Coverity runs, and define MIN_CONST and MAX_CONST to the
"classic" ternary-operator-based definitions of minimum and maximum.
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/osdep.h | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 0d26a1b9bd..4f4d4a3060 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -258,22 +258,29 @@ extern int daemon(int, int);
typeof(1 ? (a) : (b)) _a = (a), _b = (b); \
_a < _b ? _a : _b; \
})
-#define MIN_CONST(a, b) \
- __builtin_choose_expr( \
- __builtin_constant_p(a) && __builtin_constant_p(b), \
- (a) < (b) ? (a) : (b), \
- ((void)0))
+
#undef MAX
#define MAX(a, b) \
({ \
typeof(1 ? (a) : (b)) _a = (a), _b = (b); \
_a > _b ? _a : _b; \
})
+
+#ifdef __COVERITY__
+#define MIN_CONST(a, b) (a) < (b) ? (a) : (b)
+#define MAX_CONST(a, b) (a) > (b) ? (a) : (b)
+#else
+#define MIN_CONST(a, b) \
+ __builtin_choose_expr( \
+ __builtin_constant_p(a) && __builtin_constant_p(b), \
+ (a) < (b) ? (a) : (b), \
+ ((void)0))
#define MAX_CONST(a, b) \
__builtin_choose_expr( \
__builtin_constant_p(a) && __builtin_constant_p(b), \
(a) > (b) ? (a) : (b), \
((void)0))
+#endif
/*
* Minimum function that returns zero only if both values are zero.
--
2.26.2
Patchew URL: https://patchew.org/QEMU/20200629151642.11974-1-pbonzini@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH] coverity: provide Coverity-friendly MIN_CONST and MAX_CONST Type: series Message-id: 20200629151642.11974-1-pbonzini@redhat.com === 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/20200629140938.17566-1-drjones@redhat.com -> patchew/20200629140938.17566-1-drjones@redhat.com - [tag update] patchew/20200629151642.11974-1-pbonzini@redhat.com -> patchew/20200629151642.11974-1-pbonzini@redhat.com Switched to a new branch 'test' b4e52a9 coverity: provide Coverity-friendly MIN_CONST and MAX_CONST === OUTPUT BEGIN === WARNING: architecture specific defines should be avoided #39: FILE: include/qemu/osdep.h:269: +#ifdef __COVERITY__ ERROR: Macros with complex values should be enclosed in parenthesis #40: FILE: include/qemu/osdep.h:270: +#define MIN_CONST(a, b) (a) < (b) ? (a) : (b) ERROR: Macros with complex values should be enclosed in parenthesis #41: FILE: include/qemu/osdep.h:271: +#define MAX_CONST(a, b) (a) > (b) ? (a) : (b) total: 2 errors, 1 warnings, 34 lines checked Commit b4e52a97baed (coverity: provide Coverity-friendly MIN_CONST and MAX_CONST) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20200629151642.11974-1-pbonzini@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On 6/29/20 10:16 AM, Paolo Bonzini wrote:
> Coverity's parser chokes on __builtin_choose_expr inside a constant
> expression. Since it is used only to raise compilation errors for
> non-constant arguments, we can just assume there are no such errors
> in the Coverity runs, and define MIN_CONST and MAX_CONST to the
> "classic" ternary-operator-based definitions of minimum and maximum.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> include/qemu/osdep.h | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
I wrote a variant in the meantime, and in comparing the two, the only
major difference was that I added a line:
Fixes: CID 1429992, CID 1429995, CID 1429997, CID 1429999
in the commit message, as well as a comment in osdep.h:
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 0d26a1b9bd07..98bc7156fa9b 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -250,7 +250,8 @@ extern int daemon(int, int);
* Note that neither form is usable as an #if condition; if you truly
* need to write conditional code that depends on a minimum or maximum
* determined by the pre-processor instead of the compiler, you'll
- * have to open-code it.
+ * have to open-code it. Sadly, Coverity is severely confused by the
+ * constant variants, so we have to dumb things down there.
*/
#undef MIN
#define MIN(a, b) \
I'm fine whether or not we include that.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
On Mon, 29 Jun 2020 at 16:34, Eric Blake <eblake@redhat.com> wrote: > > On 6/29/20 10:16 AM, Paolo Bonzini wrote: > > Coverity's parser chokes on __builtin_choose_expr inside a constant > > expression. Since it is used only to raise compilation errors for > > non-constant arguments, we can just assume there are no such errors > > in the Coverity runs, and define MIN_CONST and MAX_CONST to the > > "classic" ternary-operator-based definitions of minimum and maximum. > > > > Reported-by: Peter Maydell <peter.maydell@linaro.org> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > --- > > include/qemu/osdep.h | 17 ++++++++++++----- > > 1 file changed, 12 insertions(+), 5 deletions(-) > > Reviewed-by: Eric Blake <eblake@redhat.com> > > I wrote a variant in the meantime, and in comparing the two, the only > major difference was that I added a line: > > Fixes: CID 1429992, CID 1429995, CID 1429997, CID 1429999 > > in the commit message, as well as a comment in osdep.h: > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 0d26a1b9bd07..98bc7156fa9b 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -250,7 +250,8 @@ extern int daemon(int, int); > * Note that neither form is usable as an #if condition; if you truly > * need to write conditional code that depends on a minimum or maximum > * determined by the pre-processor instead of the compiler, you'll > - * have to open-code it. > + * have to open-code it. Sadly, Coverity is severely confused by the > + * constant variants, so we have to dumb things down there. > */ > #undef MIN > #define MIN(a, b) \ > > > I'm fine whether or not we include that. I would vote for including the comment improvement, please. -- PMM
On 6/29/20 10:34 AM, Eric Blake wrote: > On 6/29/20 10:16 AM, Paolo Bonzini wrote: >> Coverity's parser chokes on __builtin_choose_expr inside a constant >> expression. Since it is used only to raise compilation errors for >> non-constant arguments, we can just assume there are no such errors >> in the Coverity runs, and define MIN_CONST and MAX_CONST to the >> "classic" ternary-operator-based definitions of minimum and maximum. >> >> Reported-by: Peter Maydell <peter.maydell@linaro.org> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> include/qemu/osdep.h | 17 ++++++++++++----- >> 1 file changed, 12 insertions(+), 5 deletions(-) > > Reviewed-by: Eric Blake <eblake@redhat.com> > > I wrote a variant in the meantime, and in comparing the two, the only > major difference was that I added a line: > > Fixes: CID 1429992, CID 1429995, CID 1429997, CID 1429999 > > in the commit message, as well as a comment in osdep.h: > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 0d26a1b9bd07..98bc7156fa9b 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -250,7 +250,8 @@ extern int daemon(int, int); > * Note that neither form is usable as an #if condition; if you truly > * need to write conditional code that depends on a minimum or maximum > * determined by the pre-processor instead of the compiler, you'll > - * have to open-code it. > + * have to open-code it. Sadly, Coverity is severely confused by the > + * constant variants, so we have to dumb things down there. > */ > #undef MIN > #define MIN(a, b) \ > > Oh, and the more significant difference that the syntax checker is right: we want the fallback definition to be enclosed in an outer (). Looks like we need v2. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Patchew URL: https://patchew.org/QEMU/20200629151642.11974-1-pbonzini@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH] coverity: provide Coverity-friendly MIN_CONST and MAX_CONST Type: series Message-id: 20200629151642.11974-1-pbonzini@redhat.com === 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 === From https://github.com/patchew-project/qemu * [new tag] patchew/20200629151642.11974-1-pbonzini@redhat.com -> patchew/20200629151642.11974-1-pbonzini@redhat.com Switched to a new branch 'test' 3aee0de coverity: provide Coverity-friendly MIN_CONST and MAX_CONST === OUTPUT BEGIN === WARNING: architecture specific defines should be avoided #38: FILE: include/qemu/osdep.h:269: +#ifdef __COVERITY__ ERROR: Macros with complex values should be enclosed in parenthesis #39: FILE: include/qemu/osdep.h:270: +#define MIN_CONST(a, b) (a) < (b) ? (a) : (b) ERROR: Macros with complex values should be enclosed in parenthesis #40: FILE: include/qemu/osdep.h:271: +#define MAX_CONST(a, b) (a) > (b) ? (a) : (b) total: 2 errors, 1 warnings, 34 lines checked Commit 3aee0de30ad2 (coverity: provide Coverity-friendly MIN_CONST and MAX_CONST) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20200629151642.11974-1-pbonzini@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On 6/29/20 10:21 AM, no-reply@patchew.org wrote: > Patchew URL: https://patchew.org/QEMU/20200629151642.11974-1-pbonzini@redhat.com/ > > > > Hi, > > This series seems to have some coding style problems. See output below for > more information: > > Subject: [PATCH] coverity: provide Coverity-friendly MIN_CONST and MAX_CONST > Type: series > Message-id: 20200629151642.11974-1-pbonzini@redhat.com > > === 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 === > > From https://github.com/patchew-project/qemu > * [new tag] patchew/20200629151642.11974-1-pbonzini@redhat.com -> patchew/20200629151642.11974-1-pbonzini@redhat.com > Switched to a new branch 'test' > 3aee0de coverity: provide Coverity-friendly MIN_CONST and MAX_CONST > > === OUTPUT BEGIN === > WARNING: architecture specific defines should be avoided > #38: FILE: include/qemu/osdep.h:269: > +#ifdef __COVERITY__ > > ERROR: Macros with complex values should be enclosed in parenthesis Obvious false positive. > #39: FILE: include/qemu/osdep.h:270: > +#define MIN_CONST(a, b) (a) < (b) ? (a) : (b) > > ERROR: Macros with complex values should be enclosed in parenthesis But this one is a real complaint. We really do want: #define MIN_CONST(a, b) ((a) < (b) ? (a) : (b)) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
© 2016 - 2024 Red Hat, Inc.