Re: [GIT PULL] Char/Misc driver changes for 6.9-rc1

Re: [GIT PULL] Char/Misc driver changes for 6.9-rc1

[Nathan Chancellor]

On Thu, Mar 21, 2024 at 02:02:19PM +0100, Greg KH wrote:
> All of these have been in linux-next for a long time with no reported
> issue, other than a build warning with some older versions of gcc for a
> speakup driver, fix for that will come in a few days when I catch up
> with my pending patch queues.
...
> Samuel Thibault (2):
>       speakup: Fix 8bit characters from direct synth
>       speakup: Add /dev/synthu device

That build warning actually happens with clang, not GCC as far as I am
aware, and it is actually a hard build error with older versions of
clang, as Arnd points out in his patch to fix this (although the warning
is a hard error with CONFIG_WERROR too, which causes allmodconfig to
break):

https://lore.kernel.org/20240313100413.875280-1-arnd@kernel.org/

Samuel's patch was even simpler:

https://lore.kernel.org/20240309203549.jj2l6epnznyjsrje@begin/

Why was one of these changes not applied before this was sent? I am
aware you were on vacation recently but you are now adding a known issue
to -rc1, which can proliferate to other maintainer's trees and makes
testing for us more difficult :/ -next has been broken for the entire
merge window over this, which is usually when there is a chance we can
get maybe a week of green builds...

Cheers,
Nathan

Re: [GIT PULL] Char/Misc driver changes for 6.9-rc1

[Greg KH]

On Thu, Mar 21, 2024 at 06:48:31AM -0700, Nathan Chancellor wrote:
> On Thu, Mar 21, 2024 at 02:02:19PM +0100, Greg KH wrote:
> > All of these have been in linux-next for a long time with no reported
> > issue, other than a build warning with some older versions of gcc for a
> > speakup driver, fix for that will come in a few days when I catch up
> > with my pending patch queues.
> ...
> > Samuel Thibault (2):
> >       speakup: Fix 8bit characters from direct synth
> >       speakup: Add /dev/synthu device
> 
> That build warning actually happens with clang, not GCC as far as I am
> aware, and it is actually a hard build error with older versions of
> clang, as Arnd points out in his patch to fix this (although the warning
> is a hard error with CONFIG_WERROR too, which causes allmodconfig to
> break):
> 
> https://lore.kernel.org/20240313100413.875280-1-arnd@kernel.org/
> 
> Samuel's patch was even simpler:
> 
> https://lore.kernel.org/20240309203549.jj2l6epnznyjsrje@begin/
> 
> Why was one of these changes not applied before this was sent? I am
> aware you were on vacation recently but you are now adding a known issue
> to -rc1, which can proliferate to other maintainer's trees and makes
> testing for us more difficult :/

You answered it yourself here, I'm supposed to still be on vacation and
this came in while I was supposed to be ignoring emails, AND it works
fine for me here, which is why I didn't queue it up to my tree for
inclusion in this pull request.

And sorry for the confusion with gcc/clang, I got that mixed up.

> -next has been broken for the entire
> merge window over this, which is usually when there is a chance we can
> get maybe a week of green builds...

Stuff will always slip up at times, given the low-stakes of this, I
didn't think it was needed right now, again, because my local testing
was just fine.  I can go add it to the queue or if it affects Linus's
builds, he can take one of the above changes before I get a chance to
get through my todo mbox:

	$ mdfrm -c ~/mail/todo/
	1776 messages in /home/gregkh/mail/todo/

thanks,

greg "I need a vacation from my vacation..." k-h

[PATCH] speakup: Fix warning for label at end of compound statement

[Greg KH]

From: Samuel Thibault <samuel.thibault@ens-lyon.org>

Label at end of compound statements is a C2x extension, so add an empty instruction.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reported-by: kernel test robot <lkp@intel.com>
Fixes: 807977260ae4 ("speakup: Add /dev/synthu device")
Closes: https://lore.kernel.org/oe-kbuild-all/202403090122.cpUNsozM-lkp@intel.com/
Link: https://lore.kernel.org/r/20240309203549.jj2l6epnznyjsrje@begin
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Linus,

  Here's the fix for clang on top of the char-misc-next branch if you
  pull from it, or I can integrate it in and send a new pull request,
  your choice.

  thanks,

  greg k-h

 drivers/accessibility/speakup/devsynth.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/accessibility/speakup/devsynth.c b/drivers/accessibility/speakup/devsynth.c
index da4d0f6aa5bf..674204ee5a85 100644
--- a/drivers/accessibility/speakup/devsynth.c
+++ b/drivers/accessibility/speakup/devsynth.c
@@ -108,6 +108,7 @@ static ssize_t speakup_file_writeu(struct file *fp, const char __user *buffer,
 				break;
 			}
 drop:
+			;
 		}
 
 		count -= bytes;
-- 
2.44.0

Re: [GIT PULL] Char/Misc driver changes for 6.9-rc1

[Linus Torvalds]

On Thu, 21 Mar 2024 at 06:48, Nathan Chancellor <nathan@kernel.org> wrote:
>
> That build warning actually happens with clang, not GCC as far as I am
> aware, and it is actually a hard build error with older versions of
> clang

So the "labels without a statement" thing is not only a long-time gcc
behavior (admittedly due to a parsing bug), afaik it's becoming
"standard C" in C23.

Does clang have a flag to allow this?

Considering that gcc doesn't warn for it, and that it will become
official at some point anyway, I think this might be a thing that we
might be better off just accepting, rather than be in the situation
where people write code that compiles fine with gcc and don't notice
that clang will error out.

So yes, clang is being correct, but in this case it only causes problems.

              Linus

Re: [GIT PULL] Char/Misc driver changes for 6.9-rc1

[Linus Torvalds]

On Thu, 21 Mar 2024 at 11:10, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> So the "labels without a statement" thing is not only a long-time gcc
> behavior (admittedly due to a parsing bug), afaik it's becoming
> "standard C" in C23.

Actually, let me take that back. I think it's only a proposal (WG14
N2508), I have no idea if it's actually going to be standard.

            Linus

Re: [GIT PULL] Char/Misc driver changes for 6.9-rc1

[Nathan Chancellor]

On Thu, Mar 21, 2024 at 11:10:22AM -0700, Linus Torvalds wrote:
> On Thu, 21 Mar 2024 at 06:48, Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > That build warning actually happens with clang, not GCC as far as I am
> > aware, and it is actually a hard build error with older versions of
> > clang
> 
> So the "labels without a statement" thing is not only a long-time gcc
> behavior (admittedly due to a parsing bug), afaik it's becoming
> "standard C" in C23.

Right, N2508 has been applied to the C23 draft [1] as far as I can tell
and clang finished implementing it with [2], which was released in
18.1.0.

> Does clang have a flag to allow this?

As far as I understand it, clang does not have separate flags to control
the warnings that it emits for the various c23 extensions that it allows
in older standard versions, just the big hammer of -Wc23-extensions.

Since GCC does not appear emit warnings for newer C features that it
allows even with older 'gnu' standard values by default (I think it does
with '-pedantic'?), perhaps we should just disable -Wc23-extensions
altogether? Not sure how big of a hammer this is, I think this type of
warning is the only thing I have seen come from -Wc23-extensions...

> Considering that gcc doesn't warn for it, and that it will become
> official at some point anyway, I think this might be a thing that we
> might be better off just accepting, rather than be in the situation
> where people write code that compiles fine with gcc and don't notice
> that clang will error out.
> 
> So yes, clang is being correct, but in this case it only causes problems.

Agreed, it isn't the first time :/

[1]: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf
[2]: https://github.com/llvm/llvm-project/commit/8bd06d5b65845e5e01dd899a2deb773580460b89

Cheers,
Nathan

Re: [GIT PULL] Char/Misc driver changes for 6.9-rc1

[Linus Torvalds]

On Thu, 21 Mar 2024 at 11:30, Nathan Chancellor <nathan@kernel.org> wrote:
>
> Since GCC does not appear emit warnings for newer C features that it
> allows even with older 'gnu' standard values by default (I think it does
> with '-pedantic'?), perhaps we should just disable -Wc23-extensions
> altogether? Not sure how big of a hammer this is, I think this type of
> warning is the only thing I have seen come from -Wc23-extensions...

It looks like adding -Wno-c23-extensions would only work with more
recent clang versions, so it wouldn't actually fix the build problems,
just make them even harder for developers to actually notice.

Oh well. It's not like this is all that common a problem, so I think
we'll just have to live with it, and hope that people don't do that
"label at end of statement" very often.

(I think it's case statements too, not just labels, too lazy to look
up the details again)

                 Linus