Re: [PATCH 1/4] fix not-and/or errors
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
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
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
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
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
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
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
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
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
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/
