0

This is happening in one of my C projects.

#define total 400

unsigned long values[total];

Initially the entire values array is initialised to 0xffffffff and then it is filled with some valid values. There is a use case where the entire array is filled with valid entries. In that case, i see a crash and when i tried printing i value, it is going beyond total(400).

//Crashing, i value goes beyond 400
for (i = 0; ((values[i] != 0xffffffff) && (i < total)); i++)
{
   // some processing code
}

But if i just swap the condition it works fine, it doesn't go beyond total.

//Working fine

for (i = 0; ((i < total) && (values[i] != 0xffffffff)); i++)
{
   // some processing code
}

Anyone has seen this kind of behaviour. Please shed some light if you have faced this

Scarlet
  • 131
  • 7
  • 4
    `values[0], values[1], ... values[399]` are OK to index the 400 elements of array `values[]`. `(values[i] != 0xffffffff) && (i < total)` eventually attempts `values[400]`. Do you think that is OK? What value do you think `values[400]` has as it lies outside the array `values[]`? – chux - Reinstate Monica Jul 25 '23 at 03:55
  • 1
    this is because of [short circuit evaluation](https://stackoverflow.com/questions/45848858/what-is-short-circuit-evaluation-in-c). In the 2nd snippet, `i < total` fails when `i >= 400`. Since this is ANDed with something else, it doesn't bother evaluating the 2nd condition b/c `0 && x` is always 0. In the first snippet, it doesn't know yet that `i >= total`, so it accesses `values` beyond its range, invoking undefined behavior. – yano Jul 25 '23 at 04:13

1 Answers1

2

This is short-circuiting. It's intended to improve the efficiency of code.

In your first example

((values[i] != 0xffffffff) && (i < total))

the code will attempt to evaluate the first expression, and it that is true, evaluate the second. If the first expression is false the value of the entire condition will be false, so there's no need to evaluate the second expression.

Since you're evaluating values[i], it will fail out of bounds before the bounds are checked.

Reverse the condition, as in your second example

((i < total) && (values[i] != 0xffffffff))

Now, the bounds are checked first, so the array element is not evaluated, and there is no failure.

Tangentially Perpendicular
  • 5,012
  • 4
  • 11
  • 29
  • 1
    More than efficiency, using short-circuit evaluation properly improves the safety of the code. It's important to test for a valid index before testing the value at the index. – Jonathan Leffler Jul 25 '23 at 05:00
  • Re "*It's intended to improve the efficiency of code.*", Is it, though? It's common to rely on the short-circuiting behaviour of boolean operator as more than just an optimization. (This very question is about that!) It's hard to believe that was accidental. – ikegami Jul 25 '23 at 05:03
  • If the condition fails, how is it incrementing i? shouldnt it stop ? i value keeps on getting incremented in this case – Scarlet Jul 25 '23 at 06:11