[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH] 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










[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]