public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: "Darrick J. Wong" <djwong@kernel.org>, 1000974@bugs.debian.org
Cc: Eric Sandeen <sandeen@redhat.com>,
	Thomas Goirand <zigo@debian.org>,
	Giovanni Mascellani <gio@debian.org>,
	xfslibs-dev@packages.debian.org, xfs <linux-xfs@vger.kernel.org>,
	gustavoars@kernel.org, keescook@chromium.org
Subject: Re: Bug#1000974: [PATCH xfsprogs-5.14.2 URGENT] libxfs: hide the drainbamaged fallthrough macro from xfslibs
Date: Mon, 6 Dec 2021 08:10:40 +1100	[thread overview]
Message-ID: <20211205211040.GH449541@dread.disaster.area> (raw)
In-Reply-To: <20211205174951.GQ8467@magnolia>

On Sun, Dec 05, 2021 at 09:49:51AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Back in mid-2021, Kees and Gustavo rammed into the kernel a bunch of
> static checker "improvements" that redefined '/* fallthrough */'
> comments for switch statements as a macro that virtualizes either that
> same comment, a do-while loop, or a compiler __attribute__.  This was
> necessary to work around the poor decision-making of the clang, gcc, and
> C language standard authors, who collectively came up with four mutually
> incompatible ways to document a lack of branching in a code flow.
> 
> Having received ZERO HELP porting this to userspace, Eric and I
> foolishly dumped that crap into linux.h, which was a poor decision
> because we keep forgetting that linux.h is exported as a userspace
> header.  This has now caused downstream regressions in Debian[1] and
> will probably cause more problems in the other distros.
> 
> Move it to platform_defs.h since that's not shipped publicly and leave a
> warning to anyone else who dare modify linux.h.
> 
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1000974
> 
> Fixes: df9c7d8d ("xfs: Fix fall-through warnings for Clang")
> Cc: 1000974@bugs.debian.org, gustavoars@kernel.org, keescook@chromium.org
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  include/linux.h            |   20 ++------------------
>  include/platform_defs.h.in |   21 +++++++++++++++++++++
>  2 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux.h b/include/linux.h
> index 24650228..054117aa 100644
> --- a/include/linux.h
> +++ b/include/linux.h
> @@ -360,24 +360,8 @@ fsmap_advance(
>  #endif /* HAVE_MAP_SYNC */
>  
>  /*
> - * Add the pseudo keyword 'fallthrough' so case statement blocks
> - * must end with any of these keywords:
> - *   break;
> - *   fallthrough;
> - *   continue;
> - *   goto <label>;
> - *   return [expression];
> - *
> - *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
> + * Reminder: anything added to this file will be compiled into downstream
> + * userspace projects!

This comment belongs at the top of the file before all the includes,
not at the end of it. Otherwise looks ok for a quick fix.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

But I have to wonder - why is this file even exported to userspace?
It's mostly the xfsprogs source build wrapper stuff for linux -
there's no public API in it except for xfsctl()....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com>
To: "Darrick J. Wong" <djwong@kernel.org>, 1000974@bugs.debian.org
Cc: Eric Sandeen <sandeen@redhat.com>,
	Thomas Goirand <zigo@debian.org>,
	Giovanni Mascellani <gio@debian.org>,
	xfslibs-dev@packages.debian.org, xfs <linux-xfs@vger.kernel.org>,
	gustavoars@kernel.org, keescook@chromium.org
Subject: Bug#1000974: [PATCH xfsprogs-5.14.2 URGENT] libxfs: hide the drainbamaged fallthrough macro from xfslibs
Date: Mon, 6 Dec 2021 08:10:40 +1100	[thread overview]
Message-ID: <20211205211040.GH449541@dread.disaster.area> (raw)
Message-ID: <20211205211040.Mi9ZWVc1ZfIyKnUx45wZYrhFqeZGaGvB7Jad1mP3qZA@z> (raw)
In-Reply-To: <20211205174951.GQ8467@magnolia>

On Sun, Dec 05, 2021 at 09:49:51AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Back in mid-2021, Kees and Gustavo rammed into the kernel a bunch of
> static checker "improvements" that redefined '/* fallthrough */'
> comments for switch statements as a macro that virtualizes either that
> same comment, a do-while loop, or a compiler __attribute__.  This was
> necessary to work around the poor decision-making of the clang, gcc, and
> C language standard authors, who collectively came up with four mutually
> incompatible ways to document a lack of branching in a code flow.
> 
> Having received ZERO HELP porting this to userspace, Eric and I
> foolishly dumped that crap into linux.h, which was a poor decision
> because we keep forgetting that linux.h is exported as a userspace
> header.  This has now caused downstream regressions in Debian[1] and
> will probably cause more problems in the other distros.
> 
> Move it to platform_defs.h since that's not shipped publicly and leave a
> warning to anyone else who dare modify linux.h.
> 
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1000974
> 
> Fixes: df9c7d8d ("xfs: Fix fall-through warnings for Clang")
> Cc: 1000974@bugs.debian.org, gustavoars@kernel.org, keescook@chromium.org
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  include/linux.h            |   20 ++------------------
>  include/platform_defs.h.in |   21 +++++++++++++++++++++
>  2 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux.h b/include/linux.h
> index 24650228..054117aa 100644
> --- a/include/linux.h
> +++ b/include/linux.h
> @@ -360,24 +360,8 @@ fsmap_advance(
>  #endif /* HAVE_MAP_SYNC */
>  
>  /*
> - * Add the pseudo keyword 'fallthrough' so case statement blocks
> - * must end with any of these keywords:
> - *   break;
> - *   fallthrough;
> - *   continue;
> - *   goto <label>;
> - *   return [expression];
> - *
> - *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes
> + * Reminder: anything added to this file will be compiled into downstream
> + * userspace projects!

This comment belongs at the top of the file before all the includes,
not at the end of it. Otherwise looks ok for a quick fix.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

But I have to wonder - why is this file even exported to userspace?
It's mostly the xfsprogs source build wrapper stuff for linux -
there's no public API in it except for xfsctl()....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  parent reply	other threads:[~2021-12-05 21:10 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <9b9bda73-9554-0c75-824d-f8d1b9b98e19@debian.org>
     [not found] ` <7686ac7e-0df1-a98c-27ce-51dc5e46c55e@debian.org>
     [not found]   ` <163839370805.58768.6385074074873965943.reportbug@zbuz.infomaniak.ch>
     [not found]     ` <c7ccff50-c177-7f96-2d99-2077f77374ad@debian.org>
2021-12-05 15:57       ` Processed (with 1 error): Re: Bug#1000974: copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’? Debian Bug Tracking System
2021-12-05 17:33       ` Thomas Goirand
2021-12-05 17:45         ` Darrick J. Wong
2021-12-05 17:49     ` [PATCH xfsprogs-5.14.2 URGENT] libxfs: hide the drainbamaged fallthrough macro from xfslibs Darrick J. Wong
2021-12-05 17:49       ` Bug#1000974: " Darrick J. Wong
2021-12-05 21:10       ` Dave Chinner [this message]
2021-12-05 21:10         ` Dave Chinner
2021-12-06 14:26       ` Eric Sandeen
2021-12-06 14:26         ` Bug#1000974: " Eric Sandeen
2021-12-06 20:15       ` Kees Cook
2021-12-06 20:15         ` Bug#1000974: " Kees Cook
     [not found]     ` <handler.1000974.B1000974.163872562421581.ackinfo@bugs.debian.org>
2021-12-05 19:00       ` Bug#1000974: Info received (Bug#1000974: copy_move_algo.hpp:1083:10: error: ‘__fallthrough__’ was not declared in this scope; did you mean ‘fallthrough’?) Thomas Goirand
2021-12-07 12:36     ` Bug#1000974: marked as done (xfs/linux.h defines common word "fallthrough" breaking unrelated headers) Debian Bug Tracking System
2021-12-07 12:54     ` Debian Bug Tracking System

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211205211040.GH449541@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=1000974@bugs.debian.org \
    --cc=djwong@kernel.org \
    --cc=gio@debian.org \
    --cc=gustavoars@kernel.org \
    --cc=keescook@chromium.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@redhat.com \
    --cc=xfslibs-dev@packages.debian.org \
    --cc=zigo@debian.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox