From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752063AbeDHHsb (ORCPT ); Sun, 8 Apr 2018 03:48:31 -0400 Received: from mail-wm0-f45.google.com ([74.125.82.45]:37463 "EHLO mail-wm0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751641AbeDHHsa (ORCPT ); Sun, 8 Apr 2018 03:48:30 -0400 X-Google-Smtp-Source: AIpwx49rkP8jhngFMwbHRrFNkIqnFChAeOrEHORIX2WbrgEuRdlgAmCjBgvJSbPukF8kqn+Z9LqE5g== Date: Sun, 8 Apr 2018 09:48:24 +0200 From: Luc Van Oostenryck To: Joey Pabalinas Cc: linux-sparse@vger.kernel.org, Kees Cook , Linus Torvalds , Martin Uecker , Al Viro , Christopher Li , Linux Kernel Mailing List Subject: Re: [PATCH v2] add -Wpointer-arith sparse flag to toggle sizeof(void) warnings Message-ID: <20180408074823.fgrmtrjzzjb7dlt2@ltop.local> References: <20180407215805.ze5rx4v3slbjngmo@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180407215805.ze5rx4v3slbjngmo@gmail.com> User-Agent: NeoMutt/20171215 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Apr 07, 2018 at 11:58:05AM -1000, Joey Pabalinas wrote: > Recent changes to the min()/max() macros in include/linux/kernel.h > have added a lot of noise when compiling the kernel with Sparse checking > enabled. This mostly is due to the *huge* increase in the number of > sizeof(void) warnings, a larger number of which can safely be ignored. > > Add the -Wpointer-arith flag to enable/disable these warnings (along > with the warning when applying sizeof to function types exactly like the > GCC -Wpointer-arith flag) on demand; the warning itself has been disabled > by default to reduce the large influx of noise which was inadvertently > added by commit 3c8ba0d61d04ced9f8 (kernel.h: Retain constant expression > output for max()/min()). Thanks. It's good to me. I just have a few remarks/suggestions: With the warning disabled by default (for the moment), I had to adapt the testsuite with: diff --git a/validation/sizeof-function.c b/validation/sizeof-function.c index 27d535d4e..8ff67f214 100644 --- a/validation/sizeof-function.c +++ b/validation/sizeof-function.c @@ -35,7 +35,7 @@ int test(void) /* * check-name: sizeof-function - * check-command: sparse -Wno-decl $file + * check-command: sparse -Wpointer-arith -Wno-decl $file * * check-error-start sizeof-function.c:22:14: warning: expression using sizeof on a function > diff --git a/sparse.1 b/sparse.1 > index 88343f3170f195297a..4379406999c94b806e 100644 > --- a/sparse.1 > +++ b/sparse.1 > @@ -288,6 +288,25 @@ Standard C syntax does not permit a parenthesized string as an array > initializer. GCC allows this syntax as an extension. With > \fB\-Wparen\-string\fR, Sparse will warn about this syntax. > > +Sparse does not issue these warnings by default. > +. > +.TP > +.B \-Wpointer-arith > +Warn about anything that depends on the \fBsizeof\fR a function type or of void. Maybe it would be useful to add something along the line of "like directly using the sizeof operator on void or doing pointer arithmetic on a void pointer" ? > +Although non-standard (and somewhat illogical), constructs such as \fBsizeof(void)\fR > +are often useful when the intent is to operate on an expression without evaluating > +it, e.g. in the following integer constant expression predicate: > +.nf > +#define __is_constexpr(x) \\ > + (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8))) > +.fi I think that pointer arithmetic is much more useful than taking the size of void (being able to take the size of *any* thing is somewhere in the middle, IMO). But in all case, I don't think this part should belong to the man page. Cheers, -- Luc