public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH] Makefile: check for cppcheck's --check-level option in cppcheck target
@ 2024-02-27 16:29 Stefano Brivio
  2024-02-27 21:24 ` David Gibson
  0 siblings, 1 reply; 3+ messages in thread
From: Stefano Brivio @ 2024-02-27 16:29 UTC (permalink / raw)
  To: passt-dev; +Cc: Rahil Bhimjiani, David Gibson

Don't run cppcheck to find out if the --check-level=exhaustive option
is available, unless we're actually going to run cppcheck later.

To avoid this, move this check under the cppcheck target, and
implement it in shell script instead of using Makefile directives,
because we can't easily implement conditionals in recipes.

Reported-by: Rahil Bhimjiani <me@rahil.website>
Link: https://bugs.gentoo.org/920795
Fixes: 8640d62af719 ("cppcheck: Use "exhaustive" level checking when available")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 Makefile | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index af4fa87..75d0bf3 100644
--- a/Makefile
+++ b/Makefile
@@ -287,17 +287,17 @@ clang-tidy: $(SRCS) $(HEADERS)
 	-config='{CheckOptions: [{key: bugprone-suspicious-string-compare.WarnOnImplicitComparison, value: "false"}]}' \
 	--warnings-as-errors=* $(SRCS) -- $(filter-out -pie,$(FLAGS) $(CFLAGS) $(CPPFLAGS)) -DCLANG_TIDY_58992
 
-CPPCHECK_EXHAUSTIVE :=
-ifeq ($(shell cppcheck --check-level=exhaustive /dev/null > /dev/null 2>&1; echo $$?),0)
-	CPPCHECK_EXHAUSTIVE += --check-level=exhaustive
-endif
-
 SYSTEM_INCLUDES := /usr/include $(wildcard /usr/include/$(TARGET))
 ifeq ($(shell $(CC) -v 2>&1 | grep -c "gcc version"),1)
 VER := $(shell $(CC) -dumpversion)
 SYSTEM_INCLUDES += /usr/lib/gcc/$(TARGET)/$(VER)/include
 endif
 cppcheck: $(SRCS) $(HEADERS)
+	if cppcheck --check-level=exhaustive /dev/null > /dev/null 2>&1; then \
+		CPPCHECK_EXHAUSTIVE="--check-level=exhaustive";		\
+	else								\
+		CPPCHECK_EXHAUSTIVE=;					\
+	fi;								\
 	cppcheck --std=c11 --error-exitcode=1 --enable=all --force	\
 	--inconclusive --library=posix --quiet				\
 	$(CPPCHECK_EXHAUSTIVE)						\
-- 
@@ -287,17 +287,17 @@ clang-tidy: $(SRCS) $(HEADERS)
 	-config='{CheckOptions: [{key: bugprone-suspicious-string-compare.WarnOnImplicitComparison, value: "false"}]}' \
 	--warnings-as-errors=* $(SRCS) -- $(filter-out -pie,$(FLAGS) $(CFLAGS) $(CPPFLAGS)) -DCLANG_TIDY_58992
 
-CPPCHECK_EXHAUSTIVE :=
-ifeq ($(shell cppcheck --check-level=exhaustive /dev/null > /dev/null 2>&1; echo $$?),0)
-	CPPCHECK_EXHAUSTIVE += --check-level=exhaustive
-endif
-
 SYSTEM_INCLUDES := /usr/include $(wildcard /usr/include/$(TARGET))
 ifeq ($(shell $(CC) -v 2>&1 | grep -c "gcc version"),1)
 VER := $(shell $(CC) -dumpversion)
 SYSTEM_INCLUDES += /usr/lib/gcc/$(TARGET)/$(VER)/include
 endif
 cppcheck: $(SRCS) $(HEADERS)
+	if cppcheck --check-level=exhaustive /dev/null > /dev/null 2>&1; then \
+		CPPCHECK_EXHAUSTIVE="--check-level=exhaustive";		\
+	else								\
+		CPPCHECK_EXHAUSTIVE=;					\
+	fi;								\
 	cppcheck --std=c11 --error-exitcode=1 --enable=all --force	\
 	--inconclusive --library=posix --quiet				\
 	$(CPPCHECK_EXHAUSTIVE)						\
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Makefile: check for cppcheck's --check-level option in cppcheck target
  2024-02-27 16:29 [PATCH] Makefile: check for cppcheck's --check-level option in cppcheck target Stefano Brivio
@ 2024-02-27 21:24 ` David Gibson
  2024-02-28  6:23   ` Stefano Brivio
  0 siblings, 1 reply; 3+ messages in thread
From: David Gibson @ 2024-02-27 21:24 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: passt-dev, Rahil Bhimjiani

[-- Attachment #1: Type: text/plain, Size: 2479 bytes --]

On Tue, Feb 27, 2024 at 05:29:47PM +0100, Stefano Brivio wrote:
> Don't run cppcheck to find out if the --check-level=exhaustive option
> is available, unless we're actually going to run cppcheck later.
> 
> To avoid this, move this check under the cppcheck target, and
> implement it in shell script instead of using Makefile directives,
> because we can't easily implement conditionals in recipes.
> 
> Reported-by: Rahil Bhimjiani <me@rahil.website>
> Link: https://bugs.gentoo.org/920795
> Fixes: 8640d62af719 ("cppcheck: Use "exhaustive" level checking when available")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  Makefile | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index af4fa87..75d0bf3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -287,17 +287,17 @@ clang-tidy: $(SRCS) $(HEADERS)
>  	-config='{CheckOptions: [{key: bugprone-suspicious-string-compare.WarnOnImplicitComparison, value: "false"}]}' \
>  	--warnings-as-errors=* $(SRCS) -- $(filter-out -pie,$(FLAGS) $(CFLAGS) $(CPPFLAGS)) -DCLANG_TIDY_58992
>  
> -CPPCHECK_EXHAUSTIVE :=
> -ifeq ($(shell cppcheck --check-level=exhaustive /dev/null > /dev/null 2>&1; echo $$?),0)
> -	CPPCHECK_EXHAUSTIVE += --check-level=exhaustive
> -endif
> -
>  SYSTEM_INCLUDES := /usr/include $(wildcard /usr/include/$(TARGET))
>  ifeq ($(shell $(CC) -v 2>&1 | grep -c "gcc version"),1)
>  VER := $(shell $(CC) -dumpversion)
>  SYSTEM_INCLUDES += /usr/lib/gcc/$(TARGET)/$(VER)/include
>  endif
>  cppcheck: $(SRCS) $(HEADERS)
> +	if cppcheck --check-level=exhaustive /dev/null > /dev/null 2>&1; then \
> +		CPPCHECK_EXHAUSTIVE="--check-level=exhaustive";		\
> +	else								\
> +		CPPCHECK_EXHAUSTIVE=;					\
> +	fi;								\
>  	cppcheck --std=c11 --error-exitcode=1 --enable=all --force	\
>  	--inconclusive --library=posix --quiet				\
>  	$(CPPCHECK_EXHAUSTIVE)						\

IIUC, this is essentially moving CPPCHECK_EXHAUSTIVE from a make
variable to a shell variable in the shell that make invokes.  Which is
fine, but I believe $(CPPCHECK_EXHAUSTIVE) will only expand to a make
variable.  To get the shell variable you'll need
$${CPPCHECK_EXHAUSTIVE}.  I suspect as written, this will always
expand empty.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Makefile: check for cppcheck's --check-level option in cppcheck target
  2024-02-27 21:24 ` David Gibson
@ 2024-02-28  6:23   ` Stefano Brivio
  0 siblings, 0 replies; 3+ messages in thread
From: Stefano Brivio @ 2024-02-28  6:23 UTC (permalink / raw)
  To: David Gibson; +Cc: passt-dev, Rahil Bhimjiani

On Wed, 28 Feb 2024 08:24:49 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Feb 27, 2024 at 05:29:47PM +0100, Stefano Brivio wrote:
> > Don't run cppcheck to find out if the --check-level=exhaustive option
> > is available, unless we're actually going to run cppcheck later.
> > 
> > To avoid this, move this check under the cppcheck target, and
> > implement it in shell script instead of using Makefile directives,
> > because we can't easily implement conditionals in recipes.
> > 
> > Reported-by: Rahil Bhimjiani <me@rahil.website>
> > Link: https://bugs.gentoo.org/920795
> > Fixes: 8640d62af719 ("cppcheck: Use "exhaustive" level checking when available")
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > ---
> >  Makefile | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index af4fa87..75d0bf3 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -287,17 +287,17 @@ clang-tidy: $(SRCS) $(HEADERS)
> >  	-config='{CheckOptions: [{key: bugprone-suspicious-string-compare.WarnOnImplicitComparison, value: "false"}]}' \
> >  	--warnings-as-errors=* $(SRCS) -- $(filter-out -pie,$(FLAGS) $(CFLAGS) $(CPPFLAGS)) -DCLANG_TIDY_58992
> >  
> > -CPPCHECK_EXHAUSTIVE :=
> > -ifeq ($(shell cppcheck --check-level=exhaustive /dev/null > /dev/null 2>&1; echo $$?),0)
> > -	CPPCHECK_EXHAUSTIVE += --check-level=exhaustive
> > -endif
> > -
> >  SYSTEM_INCLUDES := /usr/include $(wildcard /usr/include/$(TARGET))
> >  ifeq ($(shell $(CC) -v 2>&1 | grep -c "gcc version"),1)
> >  VER := $(shell $(CC) -dumpversion)
> >  SYSTEM_INCLUDES += /usr/lib/gcc/$(TARGET)/$(VER)/include
> >  endif
> >  cppcheck: $(SRCS) $(HEADERS)
> > +	if cppcheck --check-level=exhaustive /dev/null > /dev/null 2>&1; then \
> > +		CPPCHECK_EXHAUSTIVE="--check-level=exhaustive";		\
> > +	else								\
> > +		CPPCHECK_EXHAUSTIVE=;					\
> > +	fi;								\
> >  	cppcheck --std=c11 --error-exitcode=1 --enable=all --force	\
> >  	--inconclusive --library=posix --quiet				\
> >  	$(CPPCHECK_EXHAUSTIVE)						\  
> 
> IIUC, this is essentially moving CPPCHECK_EXHAUSTIVE from a make
> variable to a shell variable in the shell that make invokes.  Which is
> fine, but I believe $(CPPCHECK_EXHAUSTIVE) will only expand to a make
> variable.  To get the shell variable you'll need
> $${CPPCHECK_EXHAUSTIVE}.  I suspect as written, this will always
> expand empty.

Oops, you're right, I had only tested the negative case. Fixed in v2.

-- 
Stefano


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-02-28  6:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-27 16:29 [PATCH] Makefile: check for cppcheck's --check-level option in cppcheck target Stefano Brivio
2024-02-27 21:24 ` David Gibson
2024-02-28  6:23   ` Stefano Brivio

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).