diff mbox

[RFC] Sparc: Remove trigraph in die_if_kernel() message.

Message ID alpine.LNX.2.00.1106071627260.8844@swampdragon.chaosbits.net
State Rejected
Delegated to: David Miller
Headers show

Commit Message

Jesper Juhl June 7, 2011, 2:38 p.m. UTC
In arch/sparc/kernel/traps_32.c::do_priv_instruction() we have this:

  die_if_kernel("Penguin instruction from Penguin mode??!?!", regs);

If I'm not mistaken, that "??!" will be taken as a trigraph for "|" by the 
preprocessor, so the final string will end up either as
  "Penguin instruction from Penguin mode|?!"
which I assume is not what we want, or as the correct string but with a 
warning about an ignored trigraph which I assume we don't want either. So, 
in order to elliminate the trigraph but keep the original string intact I 
changed it to

  die_if_kernel("Penguin instruction from Penguin mode?""?!?!", regs);

I've tested with a small test program on my x86-64 host and it behaves as 
I would expect, but I've not tested the actual code in 
arch/sparc/kernel/traps_32.c since I have no way to compile for sparc 
(which is why I submit this as a [RFC] patch).

Please take a look and apply if you agree :-)

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 traps_32.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

David Miller June 9, 2011, 10:59 p.m. UTC | #1
From: Jesper Juhl <jj@chaosbits.net>
Date: Tue, 7 Jun 2011 16:38:19 +0200 (CEST)

> In arch/sparc/kernel/traps_32.c::do_priv_instruction() we have this:
> 
>   die_if_kernel("Penguin instruction from Penguin mode??!?!", regs);
> 
> If I'm not mistaken, that "??!" will be taken as a trigraph for "|" by the 
> preprocessor, so the final string will end up either as
>   "Penguin instruction from Penguin mode|?!"
> which I assume is not what we want, or as the correct string but with a 
> warning about an ignored trigraph which I assume we don't want either. So, 
> in order to elliminate the trigraph but keep the original string intact I 
> changed it to
> 
>   die_if_kernel("Penguin instruction from Penguin mode?""?!?!", regs);
> 
> I've tested with a small test program on my x86-64 host and it behaves as 
> I would expect, but I've not tested the actual code in 
> arch/sparc/kernel/traps_32.c since I have no way to compile for sparc 
> (which is why I submit this as a [RFC] patch).
> 
> Please take a look and apply if you agree :-)
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>

Applied, thanks Jesper :-)
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Al Viro June 9, 2011, 11:03 p.m. UTC | #2
On Thu, Jun 09, 2011 at 03:59:43PM -0700, David Miller wrote:
> > If I'm not mistaken, that "??!" will be taken as a trigraph for "|" by the 
> > preprocessor, so the final string will end up either as
> >   "Penguin instruction from Penguin mode|?!"

Not without -trigraphs, which is not on by default...

> Applied, thanks Jesper :-)

BTW, idiomatic way to deal with those is to use \? instead of string concat...
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller June 9, 2011, 11:06 p.m. UTC | #3
From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Fri, 10 Jun 2011 00:03:22 +0100

> On Thu, Jun 09, 2011 at 03:59:43PM -0700, David Miller wrote:
>> > If I'm not mistaken, that "??!" will be taken as a trigraph for "|" by the 
>> > preprocessor, so the final string will end up either as
>> >   "Penguin instruction from Penguin mode|?!"
> 
> Not without -trigraphs, which is not on by default...

Oh, in that case I'll rever this.

Thanks Al.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jesper Juhl June 10, 2011, 7:51 a.m. UTC | #4
On Thu, 9 Jun 2011, David Miller wrote:

> From: Al Viro <viro@ZenIV.linux.org.uk>
> Date: Fri, 10 Jun 2011 00:03:22 +0100
> 
> > On Thu, Jun 09, 2011 at 03:59:43PM -0700, David Miller wrote:
> >> > If I'm not mistaken, that "??!" will be taken as a trigraph for "|" by the 
> >> > preprocessor, so the final string will end up either as
> >> >   "Penguin instruction from Penguin mode|?!"
> > 
> > Not without -trigraphs, which is not on by default...
> 
> Oh, in that case I'll rever this.
> 

Well, my gcc warns about it. It seems that -Wtrigraphs is enabled by 
default:

[jj@dragon src]$ gcc --version
gcc (GCC) 4.6.0 20110603 (prerelease)
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO                                                                                    
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR 
PURPOSE.

[jj@dragon src]$ cat test.c
#include <stdio.h>

int main()
{
  printf("Penguin instruction from Penguin mode??!?!");
  return 0;
}
[jj@dragon src]$ gcc test.c
test.c: In function ‘main’:
test.c:5:48: warning: trigraph ??! ignored, use -trigraphs to enable [-Wtrigraphs]


Which was why I wrote in my original submission mail :

"... which I assume is not what we want, or as the correct string but with 
a warning about an ignored trigraph which I assume we don't want either."

So either we get the wrong string (if someone enables -trigraphs) or we 
get a warning (at least with newer gcc's). Which is why I felt the patch 
made sense as a way to avoid both unfortunate results.
diff mbox

Patch

diff --git a/arch/sparc/kernel/traps_32.c b/arch/sparc/kernel/traps_32.c
index c0490c7..d99ca40 100644
--- a/arch/sparc/kernel/traps_32.c
+++ b/arch/sparc/kernel/traps_32.c
@@ -137,7 +137,7 @@  void do_priv_instruction(struct pt_regs *regs, unsigned long pc, unsigned long n
 	siginfo_t info;
 
 	if(psr & PSR_PS)
-		die_if_kernel("Penguin instruction from Penguin mode??!?!", regs);
+		die_if_kernel("Penguin instruction from Penguin mode?""?!?!", regs);
 	info.si_signo = SIGILL;
 	info.si_errno = 0;
 	info.si_code = ILL_PRVOPC;