drivers/char/sysrq.c

drivers/char/sysrq.c

[Margit Schubert-While]

In drivers/char/sysrq.c (2.4 and 2.5) we have :

         if ((key >= '0') & (key <= '9')) {
                 retval = key - '0';
         } else if ((key >= 'a') & (key <= 'z')) {

Shouldn't the "&" be (pedantically) "&&" ?

Margit 

Re: drivers/char/sysrq.c

[Jörn Engel]

On Fri, 30 May 2003 16:44:55 +0200, Margit Schubert-While wrote:
> 
> In drivers/char/sysrq.c (2.4 and 2.5) we have :
> 
>         if ((key >= '0') & (key <= '9')) {
>                 retval = key - '0';
>         } else if ((key >= 'a') & (key <= 'z')) {
> 
> Shouldn't the "&" be (pedantically) "&&" ?

It is semantically the same.  If you can show that gcc optimization
also creates the same assembler code, few people will object to a
patch.

Jörn

-- 
Measure. Don't tune for speed until you've measured, and even then
don't unless one part of the code overwhelms the rest.
-- Rob Pike

Re: drivers/char/sysrq.c

[J.A. Magallon]

On 05.30, Jörn Engel wrote:
> On Fri, 30 May 2003 16:44:55 +0200, Margit Schubert-While wrote:
> > 
> > In drivers/char/sysrq.c (2.4 and 2.5) we have :
> > 
> >         if ((key >= '0') & (key <= '9')) {
> >                 retval = key - '0';
> >         } else if ((key >= 'a') & (key <= 'z')) {
> > 
> > Shouldn't the "&" be (pedantically) "&&" ?
> 
> It is semantically the same.  If you can show that gcc optimization
> also creates the same assembler code, few people will object to a
> patch.
> 

I see a diff:
- & is bitwise and you always perform the op
- && is logical and gcc must shortcut it

I think people use & 'cause they prefer the extra argument calculation
than the branch for the shortcut (AFAIR...)

or not ?

-- 
J.A. Magallon <jamagallon@able.es>      \                 Software is like sex:
werewolf.able.es                         \           It's better when it's free
Mandrake Linux release 9.2 (Cooker) for i586
Linux 2.4.21-rc6-jam1 (gcc 3.2.3 (Mandrake Linux 9.2 3.2.3-1mdk))

Re: drivers/char/sysrq.c

[Jörn Engel]

On Fri, 30 May 2003 17:13:17 +0200, J.A. Magallon wrote:
> On 05.30, Jörn Engel wrote:
> > On Fri, 30 May 2003 16:44:55 +0200, Margit Schubert-While wrote:
> > > 
> > > In drivers/char/sysrq.c (2.4 and 2.5) we have :
> > > 
> > >         if ((key >= '0') & (key <= '9')) {
> > >                 retval = key - '0';
> > >         } else if ((key >= 'a') & (key <= 'z')) {
> > > 
> > > Shouldn't the "&" be (pedantically) "&&" ?
> > 
> > It is semantically the same.  If you can show that gcc optimization
> > also creates the same assembler code, few people will object to a
> > patch.
> > 
> 
> I see a diff:
> - & is bitwise and you always perform the op
> - && is logical and gcc must shortcut it
> 
> I think people use & 'cause they prefer the extra argument calculation
> than the branch for the shortcut (AFAIR...)

Yes, it is an optimization, nothing more.  Either code will behave the
same, but one version might be a little faster, depending on the cpu,
unless the compiler is already clever enough to do this himself.

Just wrote a small test program with both variants and tested on i386
with gcc 3.2.3 and 2.95.4, with -O2 and -Os.  The code generated was
identical in all eight cases.  So if ever this zero optimization
reduces readability of the code, write a patch and make it better.
Beats any spelling fixes.

Jörn

-- 
Eighty percent of success is showing up.
-- Woody Allen

Re: drivers/char/sysrq.c

[H. Peter Anvin]

Followup to:  <20030530151317.GA3973@werewolf.able.es>
By author:    "J.A. Magallon" <jamagallon@able.es>
In newsgroup: linux.dev.kernel
> 
> I see a diff:
> - & is bitwise and you always perform the op
> - && is logical and gcc must shortcut it
> 
> I think people use & 'cause they prefer the extra argument calculation
> than the branch for the shortcut (AFAIR...)
> 
> or not ?
> 

In this case it doesn't matter, since gcc should be able to prove the
right-hand-side is side-effect free.

	-hpa
-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
Architectures needed: ia64 m68k mips64 ppc ppc64 s390 s390x sh v850 x86-64

Re: drivers/char/sysrq.c

[Valdis.Kletnieks]

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

On Fri, 30 May 2003 18:11:49 +0200, margitsw@t-online.de (Margit Schubert-While)  said:
> I wonder if there are other places where "&" (or "|") is coded and
> "&&" (or "||") is meant (or vice-versa) where the result is NOT semantically
> the same :-)
> It'll take a good checker to sort that one!

It shouldn't be hard at all to deal with the form:

(A compare B) op (C compare D)

The scary part would be if  the right-hand size has a side-effect - then
the choice of |, & over ||, && would definitely be bug-inducing (or possibly
bug-fixing)?

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

drivers/char/sysrq.c

[Margit Schubert-While]

I wonder if there are other places where "&" (or "|") is coded and
"&&" (or "||") is meant (or vice-versa) where the result is NOT semantically
the same :-)
It'll take a good checker to sort that one!

Margit