Re: [Fwd: [PATCH 1/4] fix not-and/or errors]

2007-10-19 Thread Mauro Carvalho Chehab 
Roel,

Em Qui, 2007-10-18 às 23:20 -0500, Mike Isely escreveu:
> Regarding the pvrusb2 part of the patch:
> 
> Yup, that's definitely a bug.  Good catch.  The correct fix is the one 
> in the patch.
> 
> Reviewed-by: Mike Isely <[EMAIL PROTECTED]>

Regarding to pvrusb2 driver:
Acked-by: Mauro Carvalho Chehab <[EMAIL PROTECTED]>
> 
>   -Mike
> 
> 
> On Thu, 18 Oct 2007, Mauro Carvalho Chehab wrote:
> 
> > Mike,
> > 
> > Please take a look on this patch.
> > 
> > Cheers,
> > Mauro.
> >  Mensagem encaminhada ----
> > De: Roel Kluin <[EMAIL PROTECTED]>
> > Para: lkml 
> > Assunto: [PATCH 1/4] fix not-and/or errors
> > Data:   Wed, 17 Oct 2007 15:46:43 +0200
> > 
> > if(!x & y) should either be if(!(x & y)) or if(!x && y)
> > I made changes as seemed appropriate, but please review
> > this is against current git.
> > 
> > Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
> > ---
> > diff --git a/drivers/media/video/pvrusb2/pvrusb2-ctrl.c 
> > b/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
> > index f569b00..46f156f 100644
> > --- a/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
> > +++ b/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
> > @@ -410,7 +410,7 @@ static int parse_mtoken(const char *ptr,unsigned int 
> > len,
> > int msk;
> > *valptr = 0;
> > for (idx = 0, msk = 1; valid_bits; idx++, msk <<= 1) {
> > -   if (!msk & valid_bits) continue;
> > +   if (!(msk & valid_bits)) continue;
> > valid_bits &= ~msk;
> > if (!names[idx]) continue;
> > slen = strlen(names[idx]);
> > diff --git a/drivers/misc/asus-laptop.c b/drivers/misc/asus-laptop.c
> > index 7dce318..65c67d1 100644
> > --- a/drivers/misc/asus-laptop.c
> > +++ b/drivers/misc/asus-laptop.c
> > @@ -322,7 +322,7 @@ static void write_status(acpi_handle handle, int out, 
> > int mask)
> >  
> > switch (mask) {
> > case MLED_ON:
> > -   out = !out & 0x1;
> > +   out = !(out & 0x1);
> > break;
> > case GLED_ON:
> > out = (out & 0x1) + 1;
> > diff --git a/drivers/s390/cio/cmf.c b/drivers/s390/cio/cmf.c
> > index b960f66..6de9d7e 100644
> > --- a/drivers/s390/cio/cmf.c
> > +++ b/drivers/s390/cio/cmf.c
> > @@ -343,10 +343,10 @@ static int cmf_copy_block(struct ccw_device *cdev)
> >  
> > if (sch->schib.scsw.fctl & SCSW_FCTL_START_FUNC) {
> > /* Don't copy if a start function is in progress. */
> > -   if ((!sch->schib.scsw.actl & SCSW_ACTL_SUSPENDED) &&
> > +   if ((!(sch->schib.scsw.actl & SCSW_ACTL_SUSPENDED)) &&
> > (sch->schib.scsw.actl &
> >  (SCSW_ACTL_DEVACT | SCSW_ACTL_SCHACT)) &&
> > -   (!sch->schib.scsw.stctl & SCSW_STCTL_SEC_STATUS))
> > +   (!(sch->schib.scsw.stctl & SCSW_STCTL_SEC_STATUS)))
> > return -EBUSY;
> > }
> > cmb_data = cdev->private->cmb;
> > diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> > index 73c44cb..81943ef 100644
> > --- a/drivers/scsi/st.c
> > +++ b/drivers/scsi/st.c
> > @@ -2882,7 +2882,7 @@ static int st_int_ioctl(struct scsi_tape *STp, 
> > unsigned int cmd_in, unsigned lon
> > !(STp->use_pf & PF_TESTED)) {
> > /* Try the other possible state of Page Format 
> > if not
> >already tried */
> > -   STp->use_pf = !STp->use_pf | PF_TESTED;
> > +   STp->use_pf = !(STp->use_pf | PF_TESTED);
> > st_release_request(SRpnt);
> > SRpnt = NULL;
> > return st_int_ioctl(STp, cmd_in, arg);
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 60a8f55..b64309e 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -335,7 +335,7 @@ static void kick_khubd(struct usb_hub *hub)
> > to_usb_interface(hub->intfdev)->pm_usage_cnt = 1;
> >  
> > spin_lock_irqsave(_event_lock, flags);
> > -   if (!hub->disconnected & list_empty(>event_list)) {
> > +   if (!hub->disconnected && list_empty(>event_list)) {
> > list_add_tail(>event_list, _event_list);
> >

Re: [Fwd: [PATCH 1/4] fix not-and/or errors]

2007-10-19 Thread Mauro Carvalho Chehab 
Roel,

Em Qui, 2007-10-18 às 23:20 -0500, Mike Isely escreveu:
 Regarding the pvrusb2 part of the patch:
 
 Yup, that's definitely a bug.  Good catch.  The correct fix is the one 
 in the patch.
 
 Reviewed-by: Mike Isely [EMAIL PROTECTED]

Regarding to pvrusb2 driver:
Acked-by: Mauro Carvalho Chehab [EMAIL PROTECTED]
 
   -Mike
 
 
 On Thu, 18 Oct 2007, Mauro Carvalho Chehab wrote:
 
  Mike,
  
  Please take a look on this patch.
  
  Cheers,
  Mauro.
   Mensagem encaminhada 
  De: Roel Kluin [EMAIL PROTECTED]
  Para: lkml linux-kernel@vger.kernel.org
  Assunto: [PATCH 1/4] fix not-and/or errors
  Data:   Wed, 17 Oct 2007 15:46:43 +0200
  
  if(!x  y) should either be if(!(x  y)) or if(!x  y)
  I made changes as seemed appropriate, but please review
  this is against current git.
  
  Signed-off-by: Roel Kluin [EMAIL PROTECTED]
  ---
  diff --git a/drivers/media/video/pvrusb2/pvrusb2-ctrl.c 
  b/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
  index f569b00..46f156f 100644
  --- a/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
  +++ b/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
  @@ -410,7 +410,7 @@ static int parse_mtoken(const char *ptr,unsigned int 
  len,
  int msk;
  *valptr = 0;
  for (idx = 0, msk = 1; valid_bits; idx++, msk = 1) {
  -   if (!msk  valid_bits) continue;
  +   if (!(msk  valid_bits)) continue;
  valid_bits = ~msk;
  if (!names[idx]) continue;
  slen = strlen(names[idx]);
  diff --git a/drivers/misc/asus-laptop.c b/drivers/misc/asus-laptop.c
  index 7dce318..65c67d1 100644
  --- a/drivers/misc/asus-laptop.c
  +++ b/drivers/misc/asus-laptop.c
  @@ -322,7 +322,7 @@ static void write_status(acpi_handle handle, int out, 
  int mask)
   
  switch (mask) {
  case MLED_ON:
  -   out = !out  0x1;
  +   out = !(out  0x1);
  break;
  case GLED_ON:
  out = (out  0x1) + 1;
  diff --git a/drivers/s390/cio/cmf.c b/drivers/s390/cio/cmf.c
  index b960f66..6de9d7e 100644
  --- a/drivers/s390/cio/cmf.c
  +++ b/drivers/s390/cio/cmf.c
  @@ -343,10 +343,10 @@ static int cmf_copy_block(struct ccw_device *cdev)
   
  if (sch-schib.scsw.fctl  SCSW_FCTL_START_FUNC) {
  /* Don't copy if a start function is in progress. */
  -   if ((!sch-schib.scsw.actl  SCSW_ACTL_SUSPENDED) 
  +   if ((!(sch-schib.scsw.actl  SCSW_ACTL_SUSPENDED)) 
  (sch-schib.scsw.actl 
   (SCSW_ACTL_DEVACT | SCSW_ACTL_SCHACT)) 
  -   (!sch-schib.scsw.stctl  SCSW_STCTL_SEC_STATUS))
  +   (!(sch-schib.scsw.stctl  SCSW_STCTL_SEC_STATUS)))
  return -EBUSY;
  }
  cmb_data = cdev-private-cmb;
  diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
  index 73c44cb..81943ef 100644
  --- a/drivers/scsi/st.c
  +++ b/drivers/scsi/st.c
  @@ -2882,7 +2882,7 @@ static int st_int_ioctl(struct scsi_tape *STp, 
  unsigned int cmd_in, unsigned lon
  !(STp-use_pf  PF_TESTED)) {
  /* Try the other possible state of Page Format 
  if not
 already tried */
  -   STp-use_pf = !STp-use_pf | PF_TESTED;
  +   STp-use_pf = !(STp-use_pf | PF_TESTED);
  st_release_request(SRpnt);
  SRpnt = NULL;
  return st_int_ioctl(STp, cmd_in, arg);
  diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
  index 60a8f55..b64309e 100644
  --- a/drivers/usb/core/hub.c
  +++ b/drivers/usb/core/hub.c
  @@ -335,7 +335,7 @@ static void kick_khubd(struct usb_hub *hub)
  to_usb_interface(hub-intfdev)-pm_usage_cnt = 1;
   
  spin_lock_irqsave(hub_event_lock, flags);
  -   if (!hub-disconnected  list_empty(hub-event_list)) {
  +   if (!hub-disconnected  list_empty(hub-event_list)) {
  list_add_tail(hub-event_list, hub_event_list);
  wake_up(khubd_wait);
  }
  diff --git a/drivers/video/i810/i810_main.c b/drivers/video/i810/i810_main.c
  index 1a7d778..1d13dd0 100644
  --- a/drivers/video/i810/i810_main.c
  +++ b/drivers/video/i810/i810_main.c
  @@ -1476,7 +1476,7 @@ static int i810fb_cursor(struct fb_info *info, struct 
  fb_cursor *cursor)
  struct i810fb_par *par = info-par;
  u8 __iomem *mmio = par-mmio_start_virtual;
   
  -   if (!par-dev_flags  LOCKUP)
  +   if (!(par-dev_flags  LOCKUP))
  return -ENXIO;
   
  if (cursor-image.width  64 || cursor-image.height  64)
  diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
  index 4ba7f0b..ce62c15 100644
  --- a/fs/ocfs2/alloc.c
  +++ b/fs/ocfs2/alloc.c
  @@ -3946,7 +3946,7 @@ static int __ocfs2_mark_extent_written(struct inode 
  *inode,
  struct ocfs2_merge_ctxt ctxt;
  struct ocfs2_extent_list *rightmost_el;
   
  -   if (!rec-e_flags  OCFS2_EXT_UNWRITTEN) {
  +   if (!(rec-e_flags  OCFS2_EXT_UNWRITTEN

Re: [PATCH 1/4] fix not-and/or errors

2007-10-18 Thread Roel Kluin 
previously applied changes removed and changed as suggested.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/misc/asus-laptop.c b/drivers/misc/asus-laptop.c
index 7dce318..752ae26 100644
--- a/drivers/misc/asus-laptop.c
+++ b/drivers/misc/asus-laptop.c
@@ -322,7 +322,7 @@ static void write_status(acpi_handle handle, int out, int 
mask)
 
switch (mask) {
case MLED_ON:
-   out = !out & 0x1;
+   out = !out;
break;
case GLED_ON:
out = (out & 0x1) + 1;
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 73c44cb..289165e 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -2882,7 +2882,8 @@ static int st_int_ioctl(struct scsi_tape *STp, unsigned 
int cmd_in, unsigned lon
!(STp->use_pf & PF_TESTED)) {
/* Try the other possible state of Page Format 
if not
   already tried */
-   STp->use_pf = !STp->use_pf | PF_TESTED;
+   STp->use_pf ^= PF_TESTED | USE_PF; /* remove 
USE_PF, set *
+   * PF_TESTED 
*/
st_release_request(SRpnt);
SRpnt = NULL;
return st_int_ioctl(STp, cmd_in, arg);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] fix not-and/or errors

2007-10-18 Thread Roel Kluin 
Al Viro wrote:
> On Wed, Oct 17, 2007 at 03:46:43PM +0200, Roel Kluin wrote:
>> +++ b/drivers/misc/asus-laptop.c
>> @@ -322,7 +322,7 @@ static void write_status(acpi_handle handle, int out, 
>> int mask)
>>  
>>  switch (mask) {
>>  case MLED_ON:
>> -out = !out & 0x1;
>> +out = !(out & 0x1);
> 
> Not sure if that's what had been intended.

It seems to me if I look at the code, that it's intended to make a bool out of 
'out'. That's
nonsense because of the precedence the ! will turn it into a boolean before the 
& 0x1.

x = !x & y behaves like x = !x for y != 0. 
for y = 1 the behavior is even the same for x = !(x & y)
so it does not matter in this case, except for clarity. I'll make it out = !out.

> 
>> @@ -2882,7 +2882,7 @@ static int st_int_ioctl(struct scsi_tape *STp, 
>> unsigned int cmd_in, unsigned lon
>>  !(STp->use_pf & PF_TESTED)) {
>>  /* Try the other possible state of Page Format 
>> if not
>> already tried */
>> -STp->use_pf = !STp->use_pf | PF_TESTED;
>> +STp->use_pf = !(STp->use_pf | PF_TESTED);
> 
> Wrong.  This code, ugly as it is, happens to be correct.  Replacement
> isn't.  I would rewrite it as ^= PF_TESTED | USE_PF; /* remove USE_PF, set *
> * PF_TESTED */
> 
> The rest is covered by Alexey's patch and one I'd posted as followup.


ok, thanks, I'll correct and omit these in my follow up patch.

Roel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] fix not-and/or errors

2007-10-18 Thread Roel Kluin 
Al Viro wrote:
 On Wed, Oct 17, 2007 at 03:46:43PM +0200, Roel Kluin wrote:
 +++ b/drivers/misc/asus-laptop.c
 @@ -322,7 +322,7 @@ static void write_status(acpi_handle handle, int out, 
 int mask)
  
  switch (mask) {
  case MLED_ON:
 -out = !out  0x1;
 +out = !(out  0x1);
 
 Not sure if that's what had been intended.

It seems to me if I look at the code, that it's intended to make a bool out of 
'out'. That's
nonsense because of the precedence the ! will turn it into a boolean before the 
 0x1.

x = !x  y behaves like x = !x for y != 0. 
for y = 1 the behavior is even the same for x = !(x  y)
so it does not matter in this case, except for clarity. I'll make it out = !out.

 
 @@ -2882,7 +2882,7 @@ static int st_int_ioctl(struct scsi_tape *STp, 
 unsigned int cmd_in, unsigned lon
  !(STp-use_pf  PF_TESTED)) {
  /* Try the other possible state of Page Format 
 if not
 already tried */
 -STp-use_pf = !STp-use_pf | PF_TESTED;
 +STp-use_pf = !(STp-use_pf | PF_TESTED);
 
 Wrong.  This code, ugly as it is, happens to be correct.  Replacement
 isn't.  I would rewrite it as ^= PF_TESTED | USE_PF; /* remove USE_PF, set *
 * PF_TESTED */
 
 The rest is covered by Alexey's patch and one I'd posted as followup.


ok, thanks, I'll correct and omit these in my follow up patch.

Roel
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] fix not-and/or errors

2007-10-18 Thread Roel Kluin 
previously applied changes removed and changed as suggested.

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/drivers/misc/asus-laptop.c b/drivers/misc/asus-laptop.c
index 7dce318..752ae26 100644
--- a/drivers/misc/asus-laptop.c
+++ b/drivers/misc/asus-laptop.c
@@ -322,7 +322,7 @@ static void write_status(acpi_handle handle, int out, int 
mask)
 
switch (mask) {
case MLED_ON:
-   out = !out  0x1;
+   out = !out;
break;
case GLED_ON:
out = (out  0x1) + 1;
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 73c44cb..289165e 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -2882,7 +2882,8 @@ static int st_int_ioctl(struct scsi_tape *STp, unsigned 
int cmd_in, unsigned lon
!(STp-use_pf  PF_TESTED)) {
/* Try the other possible state of Page Format 
if not
   already tried */
-   STp-use_pf = !STp-use_pf | PF_TESTED;
+   STp-use_pf ^= PF_TESTED | USE_PF; /* remove 
USE_PF, set *
+   * PF_TESTED 
*/
st_release_request(SRpnt);
SRpnt = NULL;
return st_int_ioctl(STp, cmd_in, arg);

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] fix not-and/or errors

2007-10-17 Thread Al Viro 
On Wed, Oct 17, 2007 at 03:46:43PM +0200, Roel Kluin wrote:
> +++ b/drivers/misc/asus-laptop.c
> @@ -322,7 +322,7 @@ static void write_status(acpi_handle handle, int out, int 
> mask)
>  
>   switch (mask) {
>   case MLED_ON:
> - out = !out & 0x1;
> + out = !(out & 0x1);

Not sure if that's what had been intended.

> @@ -2882,7 +2882,7 @@ static int st_int_ioctl(struct scsi_tape *STp, unsigned 
> int cmd_in, unsigned lon
>   !(STp->use_pf & PF_TESTED)) {
>   /* Try the other possible state of Page Format 
> if not
>  already tried */
> - STp->use_pf = !STp->use_pf | PF_TESTED;
> + STp->use_pf = !(STp->use_pf | PF_TESTED);

Wrong.  This code, ugly as it is, happens to be correct.  Replacement
isn't.  I would rewrite it as ^= PF_TESTED | USE_PF; /* remove USE_PF, set *
  * PF_TESTED */

The rest is covered by Alexey's patch and one I'd posted as followup.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] fix not-and/or errors

2007-10-17 Thread Andreas Schwab 
Roel Kluin <[EMAIL PROTECTED]> writes:

> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index 73c44cb..81943ef 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -2882,7 +2882,7 @@ static int st_int_ioctl(struct scsi_tape *STp, unsigned 
> int cmd_in, unsigned lon
>   !(STp->use_pf & PF_TESTED)) {
>   /* Try the other possible state of Page Format 
> if not
>  already tried */
> - STp->use_pf = !STp->use_pf | PF_TESTED;
> + STp->use_pf = !(STp->use_pf | PF_TESTED);

This does not make sense.  Since PF_TESTED is non-zero the expression
will always be zero.

Looking at the context the original expression is likely to be the
intended one.  use_pf at this point can either be 0 or 1 (USE_PF), and
the result is supposed to be (1|PF_TESTED) or (0|PF_TESTED), resp.

Andreas.

-- 
Andreas Schwab, SuSE Labs, [EMAIL PROTECTED]
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] fix not-and/or errors

2007-10-17 Thread Peter Zijlstra 
On Wed, 2007-10-17 at 15:46 +0200, Roel Kluin wrote:
> if(!x & y) should either be if(!(x & y)) or if(!x && y)
> I made changes as seemed appropriate, but please review
> this is against current git.
> 
> Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>

Not being familiar with any of the code touched, the choices look good.

Acked-by: Peter Zijlstra <[EMAIL PROTECTED]>

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] fix not-and/or errors

2007-10-17 Thread Roel Kluin 
if(!x & y) should either be if(!(x & y)) or if(!x && y)
I made changes as seemed appropriate, but please review
this is against current git.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/media/video/pvrusb2/pvrusb2-ctrl.c 
b/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
index f569b00..46f156f 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
@@ -410,7 +410,7 @@ static int parse_mtoken(const char *ptr,unsigned int len,
int msk;
*valptr = 0;
for (idx = 0, msk = 1; valid_bits; idx++, msk <<= 1) {
-   if (!msk & valid_bits) continue;
+   if (!(msk & valid_bits)) continue;
valid_bits &= ~msk;
if (!names[idx]) continue;
slen = strlen(names[idx]);
diff --git a/drivers/misc/asus-laptop.c b/drivers/misc/asus-laptop.c
index 7dce318..65c67d1 100644
--- a/drivers/misc/asus-laptop.c
+++ b/drivers/misc/asus-laptop.c
@@ -322,7 +322,7 @@ static void write_status(acpi_handle handle, int out, int 
mask)
 
switch (mask) {
case MLED_ON:
-   out = !out & 0x1;
+   out = !(out & 0x1);
break;
case GLED_ON:
out = (out & 0x1) + 1;
diff --git a/drivers/s390/cio/cmf.c b/drivers/s390/cio/cmf.c
index b960f66..6de9d7e 100644
--- a/drivers/s390/cio/cmf.c
+++ b/drivers/s390/cio/cmf.c
@@ -343,10 +343,10 @@ static int cmf_copy_block(struct ccw_device *cdev)
 
if (sch->schib.scsw.fctl & SCSW_FCTL_START_FUNC) {
/* Don't copy if a start function is in progress. */
-   if ((!sch->schib.scsw.actl & SCSW_ACTL_SUSPENDED) &&
+   if ((!(sch->schib.scsw.actl & SCSW_ACTL_SUSPENDED)) &&
(sch->schib.scsw.actl &
 (SCSW_ACTL_DEVACT | SCSW_ACTL_SCHACT)) &&
-   (!sch->schib.scsw.stctl & SCSW_STCTL_SEC_STATUS))
+   (!(sch->schib.scsw.stctl & SCSW_STCTL_SEC_STATUS)))
return -EBUSY;
}
cmb_data = cdev->private->cmb;
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 73c44cb..81943ef 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -2882,7 +2882,7 @@ static int st_int_ioctl(struct scsi_tape *STp, unsigned 
int cmd_in, unsigned lon
!(STp->use_pf & PF_TESTED)) {
/* Try the other possible state of Page Format 
if not
   already tried */
-   STp->use_pf = !STp->use_pf | PF_TESTED;
+   STp->use_pf = !(STp->use_pf | PF_TESTED);
st_release_request(SRpnt);
SRpnt = NULL;
return st_int_ioctl(STp, cmd_in, arg);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 60a8f55..b64309e 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -335,7 +335,7 @@ static void kick_khubd(struct usb_hub *hub)
to_usb_interface(hub->intfdev)->pm_usage_cnt = 1;
 
spin_lock_irqsave(_event_lock, flags);
-   if (!hub->disconnected & list_empty(>event_list)) {
+   if (!hub->disconnected && list_empty(>event_list)) {
list_add_tail(>event_list, _event_list);
wake_up(_wait);
}
diff --git a/drivers/video/i810/i810_main.c b/drivers/video/i810/i810_main.c
index 1a7d778..1d13dd0 100644
--- a/drivers/video/i810/i810_main.c
+++ b/drivers/video/i810/i810_main.c
@@ -1476,7 +1476,7 @@ static int i810fb_cursor(struct fb_info *info, struct 
fb_cursor *cursor)
struct i810fb_par *par = info->par;
u8 __iomem *mmio = par->mmio_start_virtual;
 
-   if (!par->dev_flags & LOCKUP)
+   if (!(par->dev_flags & LOCKUP))
return -ENXIO;
 
if (cursor->image.width > 64 || cursor->image.height > 64)
diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
index 4ba7f0b..ce62c15 100644
--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -3946,7 +3946,7 @@ static int __ocfs2_mark_extent_written(struct inode 
*inode,
struct ocfs2_merge_ctxt ctxt;
struct ocfs2_extent_list *rightmost_el;
 
-   if (!rec->e_flags & OCFS2_EXT_UNWRITTEN) {
+   if (!(rec->e_flags & OCFS2_EXT_UNWRITTEN)) {
ret = -EIO;
mlog_errno(ret);
goto out;
diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 41c76ff..ef09fd2 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -670,7 +670,7 @@ static inline void 
ocfs2_generic_handle_attach_action(struct ocfs2_lock_res *loc
 {
mlog_entry_void();
 
-   BUG_ON((!lockres->l_flags & OCFS2_LOCK_BUSY));
+   BUG_ON((!(lockres->l_flags & OCFS2_LOCK_BUSY)));
BUG_ON(lockres->l_flags & OCFS2_LOCK_ATTACHED);
 
if (lockres->l_requested > LKM_NLMODE &&
diff --git a/net/ieee80211/ieee80211_wx.c

[PATCH 1/4] fix not-and/or errors

2007-10-17 Thread Roel Kluin 
if(!x  y) should either be if(!(x  y)) or if(!x  y)
I made changes as seemed appropriate, but please review
this is against current git.

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/drivers/media/video/pvrusb2/pvrusb2-ctrl.c 
b/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
index f569b00..46f156f 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-ctrl.c
@@ -410,7 +410,7 @@ static int parse_mtoken(const char *ptr,unsigned int len,
int msk;
*valptr = 0;
for (idx = 0, msk = 1; valid_bits; idx++, msk = 1) {
-   if (!msk  valid_bits) continue;
+   if (!(msk  valid_bits)) continue;
valid_bits = ~msk;
if (!names[idx]) continue;
slen = strlen(names[idx]);
diff --git a/drivers/misc/asus-laptop.c b/drivers/misc/asus-laptop.c
index 7dce318..65c67d1 100644
--- a/drivers/misc/asus-laptop.c
+++ b/drivers/misc/asus-laptop.c
@@ -322,7 +322,7 @@ static void write_status(acpi_handle handle, int out, int 
mask)
 
switch (mask) {
case MLED_ON:
-   out = !out  0x1;
+   out = !(out  0x1);
break;
case GLED_ON:
out = (out  0x1) + 1;
diff --git a/drivers/s390/cio/cmf.c b/drivers/s390/cio/cmf.c
index b960f66..6de9d7e 100644
--- a/drivers/s390/cio/cmf.c
+++ b/drivers/s390/cio/cmf.c
@@ -343,10 +343,10 @@ static int cmf_copy_block(struct ccw_device *cdev)
 
if (sch-schib.scsw.fctl  SCSW_FCTL_START_FUNC) {
/* Don't copy if a start function is in progress. */
-   if ((!sch-schib.scsw.actl  SCSW_ACTL_SUSPENDED) 
+   if ((!(sch-schib.scsw.actl  SCSW_ACTL_SUSPENDED)) 
(sch-schib.scsw.actl 
 (SCSW_ACTL_DEVACT | SCSW_ACTL_SCHACT)) 
-   (!sch-schib.scsw.stctl  SCSW_STCTL_SEC_STATUS))
+   (!(sch-schib.scsw.stctl  SCSW_STCTL_SEC_STATUS)))
return -EBUSY;
}
cmb_data = cdev-private-cmb;
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 73c44cb..81943ef 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -2882,7 +2882,7 @@ static int st_int_ioctl(struct scsi_tape *STp, unsigned 
int cmd_in, unsigned lon
!(STp-use_pf  PF_TESTED)) {
/* Try the other possible state of Page Format 
if not
   already tried */
-   STp-use_pf = !STp-use_pf | PF_TESTED;
+   STp-use_pf = !(STp-use_pf | PF_TESTED);
st_release_request(SRpnt);
SRpnt = NULL;
return st_int_ioctl(STp, cmd_in, arg);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 60a8f55..b64309e 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -335,7 +335,7 @@ static void kick_khubd(struct usb_hub *hub)
to_usb_interface(hub-intfdev)-pm_usage_cnt = 1;
 
spin_lock_irqsave(hub_event_lock, flags);
-   if (!hub-disconnected  list_empty(hub-event_list)) {
+   if (!hub-disconnected  list_empty(hub-event_list)) {
list_add_tail(hub-event_list, hub_event_list);
wake_up(khubd_wait);
}
diff --git a/drivers/video/i810/i810_main.c b/drivers/video/i810/i810_main.c
index 1a7d778..1d13dd0 100644
--- a/drivers/video/i810/i810_main.c
+++ b/drivers/video/i810/i810_main.c
@@ -1476,7 +1476,7 @@ static int i810fb_cursor(struct fb_info *info, struct 
fb_cursor *cursor)
struct i810fb_par *par = info-par;
u8 __iomem *mmio = par-mmio_start_virtual;
 
-   if (!par-dev_flags  LOCKUP)
+   if (!(par-dev_flags  LOCKUP))
return -ENXIO;
 
if (cursor-image.width  64 || cursor-image.height  64)
diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
index 4ba7f0b..ce62c15 100644
--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -3946,7 +3946,7 @@ static int __ocfs2_mark_extent_written(struct inode 
*inode,
struct ocfs2_merge_ctxt ctxt;
struct ocfs2_extent_list *rightmost_el;
 
-   if (!rec-e_flags  OCFS2_EXT_UNWRITTEN) {
+   if (!(rec-e_flags  OCFS2_EXT_UNWRITTEN)) {
ret = -EIO;
mlog_errno(ret);
goto out;
diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 41c76ff..ef09fd2 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -670,7 +670,7 @@ static inline void 
ocfs2_generic_handle_attach_action(struct ocfs2_lock_res *loc
 {
mlog_entry_void();
 
-   BUG_ON((!lockres-l_flags  OCFS2_LOCK_BUSY));
+   BUG_ON((!(lockres-l_flags  OCFS2_LOCK_BUSY)));
BUG_ON(lockres-l_flags  OCFS2_LOCK_ATTACHED);
 
if (lockres-l_requested  LKM_NLMODE 
diff --git a/net/ieee80211/ieee80211_wx.c b/net/ieee80211/ieee80211_wx.c
index 9b58dd6..bc67230 100644
---

Re: [PATCH 1/4] fix not-and/or errors

2007-10-17 Thread Andreas Schwab 
Roel Kluin [EMAIL PROTECTED] writes:

 diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
 index 73c44cb..81943ef 100644
 --- a/drivers/scsi/st.c
 +++ b/drivers/scsi/st.c
 @@ -2882,7 +2882,7 @@ static int st_int_ioctl(struct scsi_tape *STp, unsigned 
 int cmd_in, unsigned lon
   !(STp-use_pf  PF_TESTED)) {
   /* Try the other possible state of Page Format 
 if not
  already tried */
 - STp-use_pf = !STp-use_pf | PF_TESTED;
 + STp-use_pf = !(STp-use_pf | PF_TESTED);

This does not make sense.  Since PF_TESTED is non-zero the expression
will always be zero.

Looking at the context the original expression is likely to be the
intended one.  use_pf at this point can either be 0 or 1 (USE_PF), and
the result is supposed to be (1|PF_TESTED) or (0|PF_TESTED), resp.

Andreas.

-- 
Andreas Schwab, SuSE Labs, [EMAIL PROTECTED]
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] fix not-and/or errors

2007-10-17 Thread Peter Zijlstra 
On Wed, 2007-10-17 at 15:46 +0200, Roel Kluin wrote:
 if(!x  y) should either be if(!(x  y)) or if(!x  y)
 I made changes as seemed appropriate, but please review
 this is against current git.
 
 Signed-off-by: Roel Kluin [EMAIL PROTECTED]

Not being familiar with any of the code touched, the choices look good.

Acked-by: Peter Zijlstra [EMAIL PROTECTED]

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] fix not-and/or errors

2007-10-17 Thread Al Viro 
On Wed, Oct 17, 2007 at 03:46:43PM +0200, Roel Kluin wrote:
 +++ b/drivers/misc/asus-laptop.c
 @@ -322,7 +322,7 @@ static void write_status(acpi_handle handle, int out, int 
 mask)
  
   switch (mask) {
   case MLED_ON:
 - out = !out  0x1;
 + out = !(out  0x1);

Not sure if that's what had been intended.

 @@ -2882,7 +2882,7 @@ static int st_int_ioctl(struct scsi_tape *STp, unsigned 
 int cmd_in, unsigned lon
   !(STp-use_pf  PF_TESTED)) {
   /* Try the other possible state of Page Format 
 if not
  already tried */
 - STp-use_pf = !STp-use_pf | PF_TESTED;
 + STp-use_pf = !(STp-use_pf | PF_TESTED);

Wrong.  This code, ugly as it is, happens to be correct.  Replacement
isn't.  I would rewrite it as ^= PF_TESTED | USE_PF; /* remove USE_PF, set *
  * PF_TESTED */

The rest is covered by Alexey's patch and one I'd posted as followup.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/