Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   Debian User (http://www.linux-archive.org/debian-user/)
-   -   Fix test for CTC devices from yesterday. (http://www.linux-archive.org/debian-user/447588-fix-test-ctc-devices-yesterday.html)

Chris Lumens 11-03-2010 02:30 PM

Fix test for CTC devices from yesterday.
 
---
pyanaconda/isys/devices.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/pyanaconda/isys/devices.c b/pyanaconda/isys/devices.c
index 5d509c3..a30f006 100644
--- a/pyanaconda/isys/devices.c
+++ b/pyanaconda/isys/devices.c
@@ -172,7 +172,7 @@ storagedone:
}

/* S390 channel-to-channnel devices have type 256 */
- if (type != 1 && !strncmp(ent->d_name, "ctc", 3) && type != 256)
+ if (type != 1 || (!strncmp(ent->d_name, "ctc", 3) && type != 256))
continue;

new = calloc(1, sizeof(struct device));
--
1.7.1.1

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

Radek Vykydal 11-03-2010 02:54 PM

Fix test for CTC devices from yesterday.
 
On 11/03/2010 04:30 PM, Chris Lumens wrote:

---
pyanaconda/isys/devices.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/pyanaconda/isys/devices.c b/pyanaconda/isys/devices.c
index 5d509c3..a30f006 100644
--- a/pyanaconda/isys/devices.c
+++ b/pyanaconda/isys/devices.c
@@ -172,7 +172,7 @@ storagedone:
}

/* S390 channel-to-channnel devices have type 256 */
- if (type != 1&& !strncmp(ent->d_name, "ctc", 3)&& type != 256)
+ if (type != 1 || (!strncmp(ent->d_name, "ctc", 3)&& type != 256))
continue;

new = calloc(1, sizeof(struct device));



I feel confused, but shouldn't the condition stand:

if ( type != 1 && !(!strncmp(ent->d_name, "ctc", 3) && type == 256) )

That is: ignore the same as before except for device that has type 256
and starts with "cnc"?

(original condition was: if (type != 1))


Radek

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

David Cantrell 11-03-2010 04:36 PM

Fix test for CTC devices from yesterday.
 
On Wed, 3 Nov 2010, Radek Vykydal wrote:


On 11/03/2010 04:30 PM, Chris Lumens wrote:

---
pyanaconda/isys/devices.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/pyanaconda/isys/devices.c b/pyanaconda/isys/devices.c
index 5d509c3..a30f006 100644
--- a/pyanaconda/isys/devices.c
+++ b/pyanaconda/isys/devices.c
@@ -172,7 +172,7 @@ storagedone:
}

/* S390 channel-to-channnel devices have type 256 */
- if (type != 1&& !strncmp(ent->d_name, "ctc", 3)&& type !=
256)


Here all the expressions are and'ed.

Test 1:
type = 1
ent->d_name = "foo"

type != 1 FAIL
!strncmp(ent->d_name, "ctc", 3) FAIL
type != 256 PASS

got FAIL && FAIL && PASS = FAIL

Test 2:
type = 1
end->d_name = "ctc"

type != 1 FAIL
!strncmp(ent->d_name, "ctc", 3) PASS
type != 256 PASS

got FAIL && PASS && PASS = FAIL

Test 3:
type = 2
end->d_name = "ctc"

type != 1 PASS
!strncmp(ent->d_name, "ctc", 3) PASS
type != 256 PASS

got PASS && PASS && PASS = PASS

+ if (type != 1 || (!strncmp(ent->d_name, "ctc", 3)&& type !=
256))


Here the last two expressions are and'ed and that result is or'ed with the
first expression.

Test 1:
type = 1
ent->d_name = "foo"

type != 1 FAIL
!strncmp(ent->d_name, "ctc", 3) FAIL
&& FAIL
type != 256 PASS /

got FAIL || FAIL = FAIL

Test 2:
type = 1
end->d_name = "ctc"

type != 1 FAIL
!strncmp(ent->d_name, "ctc", 3) PASS
&& PASS
type != 256 PASS /

got FAIL || PASS = PASS

Test 3:
type = 2
end->d_name = "ctc"

type != 1 PASS
!strncmp(ent->d_name, "ctc", 3) PASS
&& PASS
type != 256 PASS /

got PASS || PASS = PASS



continue;

new = calloc(1, sizeof(struct device));



I feel confused, but shouldn't the condition stand:

if ( type != 1 && !(!strncmp(ent->d_name, "ctc", 3) && type == 256) )

That is: ignore the same as before except for device that has type 256
and starts with "cnc"?

(original condition was: if (type != 1))


We went from FAIL, FAIL, PASS to FAIL, PASS, PASS with the addition of the
parens. Without the patch, the code requires ctc devices to be type != 1 and
type != 256, when we really need to just check that type != 256 if we have a
ctc device.

--
David Cantrell <dcantrell@redhat.com>
Red Hat / Honolulu, HI

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

David Cantrell 11-03-2010 05:20 PM

Fix test for CTC devices from yesterday.
 
On Wed, 3 Nov 2010, David Cantrell wrote:


On Wed, 3 Nov 2010, Radek Vykydal wrote:


On 11/03/2010 04:30 PM, Chris Lumens wrote:

---
pyanaconda/isys/devices.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/pyanaconda/isys/devices.c b/pyanaconda/isys/devices.c
index 5d509c3..a30f006 100644
--- a/pyanaconda/isys/devices.c
+++ b/pyanaconda/isys/devices.c
@@ -172,7 +172,7 @@ storagedone:
}

/* S390 channel-to-channnel devices have type 256 */
- if (type != 1&& !strncmp(ent->d_name, "ctc", 3)&& type !=
256)


Here all the expressions are and'ed.

Test 1:
type = 1
ent->d_name = "foo"

type != 1 FAIL
!strncmp(ent->d_name, "ctc", 3) FAIL
type != 256 PASS

got FAIL && FAIL && PASS = FAIL

Test 2:
type = 1
end->d_name = "ctc"

type != 1 FAIL
!strncmp(ent->d_name, "ctc", 3) PASS
type != 256 PASS

got FAIL && PASS && PASS = FAIL

Test 3:
type = 2
end->d_name = "ctc"

type != 1 PASS
!strncmp(ent->d_name, "ctc", 3) PASS
type != 256 PASS

got PASS && PASS && PASS = PASS

+ if (type != 1 || (!strncmp(ent->d_name, "ctc", 3)&& type !=
256))


Here the last two expressions are and'ed and that result is or'ed with the
first expression.

Test 1:
type = 1
ent->d_name = "foo"

type != 1 FAIL
!strncmp(ent->d_name, "ctc", 3) FAIL
&& FAIL
type != 256 PASS /

got FAIL || FAIL = FAIL

Test 2:
type = 1
end->d_name = "ctc"

type != 1 FAIL
!strncmp(ent->d_name, "ctc", 3) PASS
&& PASS
type != 256 PASS /

got FAIL || PASS = PASS

Test 3:
type = 2
end->d_name = "ctc"

type != 1 PASS
!strncmp(ent->d_name, "ctc", 3) PASS
&& PASS
type != 256 PASS /

got PASS || PASS = PASS



continue;

new = calloc(1, sizeof(struct device));



I feel confused, but shouldn't the condition stand:

if ( type != 1 && !(!strncmp(ent->d_name, "ctc", 3) && type == 256) )

That is: ignore the same as before except for device that has type 256
and starts with "cnc"?

(original condition was: if (type != 1))


We went from FAIL, FAIL, PASS to FAIL, PASS, PASS with the addition of the
parens. Without the patch, the code requires ctc devices to be type != 1 and
type != 256, when we really need to just check that type != 256 if we have a
ctc device.



Oh, and I meant ack by all that.

--
David Cantrell <dcantrell@redhat.com>
Red Hat / Honolulu, HI

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

Radek Vykydal 11-04-2010 11:34 AM

Fix test for CTC devices from yesterday.
 
On 11/03/2010 06:36 PM, David Cantrell wrote:

On Wed, 3 Nov 2010, Radek Vykydal wrote:


On 11/03/2010 04:30 PM, Chris Lumens wrote:

---
pyanaconda/isys/devices.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/pyanaconda/isys/devices.c b/pyanaconda/isys/devices.c
index 5d509c3..a30f006 100644
--- a/pyanaconda/isys/devices.c
+++ b/pyanaconda/isys/devices.c
@@ -172,7 +172,7 @@ storagedone:
}

/* S390 channel-to-channnel devices have type 256 */
- if (type != 1&& !strncmp(ent->d_name, "ctc", 3)&&
type != 256)


Here all the expressions are and'ed.

Test 1:
type = 1
ent->d_name = "foo"

type != 1 FAIL
!strncmp(ent->d_name, "ctc", 3) FAIL
type != 256 PASS

got FAIL && FAIL && PASS = FAIL

Test 2:
type = 1
end->d_name = "ctc"

type != 1 FAIL
!strncmp(ent->d_name, "ctc", 3) PASS
type != 256 PASS

got FAIL && PASS && PASS = FAIL

Test 3:
type = 2
end->d_name = "ctc"

type != 1 PASS
!strncmp(ent->d_name, "ctc", 3) PASS
type != 256 PASS

got PASS && PASS && PASS = PASS

+ if (type != 1 || (!strncmp(ent->d_name, "ctc", 3)&&
type != 256))


Here the last two expressions are and'ed and that result is or'ed with
the

first expression.

Test 1:
type = 1
ent->d_name = "foo"

type != 1 FAIL
!strncmp(ent->d_name, "ctc", 3) FAIL
&& FAIL
type != 256 PASS /

got FAIL || FAIL = FAIL

Test 2:
type = 1
end->d_name = "ctc"

type != 1 FAIL
!strncmp(ent->d_name, "ctc", 3) PASS
&& PASS
type != 256 PASS /

got FAIL || PASS = PASS

Test 3:
type = 2
end->d_name = "ctc"

type != 1 PASS
!strncmp(ent->d_name, "ctc", 3) PASS
&& PASS
type != 256 PASS /

got PASS || PASS = PASS



continue;

new = calloc(1, sizeof(struct device));



I feel confused, but shouldn't the condition stand:

if ( type != 1 && !(!strncmp(ent->d_name, "ctc", 3) && type == 256) )

That is: ignore the same as before except for device that has type 256
and starts with "cnc"?

(original condition was: if (type != 1))


We went from FAIL, FAIL, PASS to FAIL, PASS, PASS with the addition of
the
parens. Without the patch, the code requires ctc devices to be type
!= 1 and
type != 256, when we really need to just check that type != 256 if we
have a

ctc device.



Hm, the comment in the original patch says:
/* S390 channel-to-channnel devices have type 256 */

so I think your second case is not so relevant (my proposition
would FAIL, FAIL, PASS), but following case is:

(A)
type = 256
end->d_name = "ctc"

and the goal of the original patch and the BZ (as I understood it)
was not to ignore this device - therefore FAIL.


I assume both name and type are important, so this:

(B)
type = 256
end->d_name = "foo"

should be ignored - PASS


This patch came up as a fix of the problem that
with original patch we stopped to ignore lo device:

(C)
type != 256 (don't know what exactly)
end->d_name = "lo"

with

- if (type != 1&& !strncmp(ent->d_name, "ctc", 3)&& type !=
256)


FAILS, but with

+ if (type != 1 || (!strncmp(ent->d_name, "ctc", 3)&& type
!= 256))


PASSes (that is, lo device is ignored as before).



To sum up:

expected behavior - as I understand it:
A: FAIL
B: PASS
C: PASS

- if (type != 1&& !strncmp(ent->d_name, "ctc", 3)&& type !=
256)

gives :
A: FAIL
B: FAIL
C: FAIL

+ if (type != 1 || (!strncmp(ent->d_name, "ctc", 3)&& type
!= 256)):

gives:
A: PASS
B: PASS
C: PASS

my suggestion
if ( type != 1 && !(!strncmp(ent->d_name, "ctc", 3) && type == 256) )
gives:
A: FAIL
B: PASS
C: PASS


Radek









_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

"Brian C. Lane" 11-04-2010 02:53 PM

Fix test for CTC devices from yesterday.
 
On Thu, Nov 04, 2010 at 01:34:29PM +0100, Radek Vykydal wrote:

> my suggestion
> if ( type != 1 && !(!strncmp(ent->d_name, "ctc", 3) && type == 256) )
> gives:
> A: FAIL
> B: PASS
> C: PASS

I agree with Radek here. This is how I examine these kinds of problems:


TRUE: if type != 1 and type != 256

TRUE: if type == 256 but name isn't ctc

if ((type != 1) && !((type == 256) && !strncmp(name, "ctc", 3)))
#1 #2 #3

type, name 1 !(2&&3) 2 3 Result
1 bob F T F F F skip continue
2 sam T T F F T continue
256 sam T T T F T continue
256 ctc T F T T F skip continue

I like putting the type check first because that will short-circuit the &&
preventing needless calls to strncmp. And some may say I use () excessively,
but it prevents confusion when evaluating long expressions.


--
Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list


All times are GMT. The time now is 04:19 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.