Writing bugs

[Rebel]

Today I had a new one, took me hours to see it.

While parsing a PGN file looking for the moves of a certain engine I wrote the following (pseudo) code:

Code: Select all

printf ("Type engine"); gets(player);

if ((process_pgn_file())!=0) { all done }

if (strcmp(player,white)==0) engine=0;    // decide the color
if (strcmp(player,black)==0) engine=1; 

// Processing the moves

for (x=0; x<=MAX_MOVES; x++) { 
if (x&1 != engine) continue;
do_something_with_the_move_of_player();
}

So what was wrong?

[brianr]

Well, a few of things look like potential issues.
A) the "player" might not match either one for the current game being processed.
B) MAX_MOVES looks like a constant, and not the number of moves in the current game.
C) Loop depends on how move numbers are counted (full moves v half moves).

[elpapa]

x <= MAX_MOVES
x < MAX_MOVES

[mar]

aside from what has been mentioned already, C operator precedence:
& has lower priority than != (I never understood why bitwise ops are lower than relational to be honest),
so the expression evaluates as x & (1 != engine) instead of the intended (x & 1) != engine
or maybe it was intended?

[Rebel]

aside from what has been mentioned already,

These were deliberately left out in the pseudo code but are covered in the real code, so those are not the issue.

C operator precedence:
& has lower priority than != (I never understood why bitwise ops are lower than relational to be honest),
so the expression evaluates as x & (1 != engine) instead of the intended (x & 1) != engine
or maybe it was intended?

You got it.

if (x&1 != engine) continue;

is bad C code, the x&1 only affects the flags.

The right way of doing is:

Code: Select all

y = x & 1;
if (y != engine) continue;

[hgm]

Nothing wring with

if((x&1) != engine) ...

If you dislike parentheses, you could also do

if(x-engine & 1) ...

I wouldn't say adding unnecessary dummy variables is "the right way to do it".

[Sesse]

Today I had a new one, took me hours to see it.

Why didn't your compiler warn you?

[Rebel]

Today I had a new one, took me hours to see it.

Why didn't your compiler warn you?

I asked myself the same question.

[lauriet]

I've spotted the error. You are using C.

It's like using the large hadron collide to change a flat tyre.

[Sven]

Code: Select all

if (x&1 != engine) continue;

is bad C code, the x&1 only affects the flags.

Just to make it clear for all those who are not fully aware of the consequences of this bug: the expression "x & (1 != engine)" is evaluated differently from the intended "(x & 1) != engine" if engine=1 and x is an even number (including x=0). In all three other combinations it "works by accident".