logical && in for condition seemingly ignored

Projects Programming
1

well this is driving me nuts....

I have a for loop with a condition that has two parts connected with logical AND &&

for (int i = 1 ; ((i <= MAX_CHANNELS) && (rslights[i].type != 'W')) ; ++i)

as I understand it, if the condition is false, the loop body will not execute.
Since both parts are connected with &&, both parts must be true for the overall condition to be true.
Since one part is a counter, and i is not altered in the body or the other condition, this loop should never iterate more than MAX_CHANNELS which is 5.

What happens though is that the loop iterates way more than 5 - usually until reading of memory gets nasty and an exception is thrown.

In my mind, at most, 5 iterations should happen, no matter what (rslights[ i ].type != 'W') evaluates to. If I alter the condition to compare it with data that is in the array, the loop works. in other words, once it evaluates as true, it seems to affect the left hand condition as if the two were connected with logical OR ||

I am certain I don't need an OR, it has to be an AND, as I want the loop to end if either condition is false.

Now, the condition does involve a variable that is a struct. If is use a plain char variable it works as expected.

I am able to access rslights[ i ].type within the body and results are as expected. Do members of a struct have to have some special way of referencing them in a logical condition?

Here is a sample sketch demonstrating the issue...

#include "ESP8266WiFi.h"
const int MAX_CHANNELS = 5;  // size of rslights array

struct RSLight {
  // design time values
  char type;   // L = monochrome light   
  int  pin;   // raw pin number 
 
};

RSLight rslights[MAX_CHANNELS];

void sendCHdata() {
  Serial.println(MAX_CHANNELS);
  for (int i = 1 ; ((i <= MAX_CHANNELS) && (rslights[i].type != 'W')) ; ++i) {     
           Serial.println(rslights[i].type); 
           Serial.println(i);
          }
}
void setup() {

 Serial.begin(115200);
 delay(1000);
 Serial.println("setup");
 rslights[1].type = 'X' ;
 rslights[2].type = 'Y' ;
 rslights[3].type = 'Z' ;
 rslights[4].type = 'P' ;
 rslights[5].type = 'Q' ;
sendCHdata();
}

void loop() {

}

and some of the Serial output (which ran until i == 2952 and crashed.)

setup
5
X
1
Y
2
Z
3
P
4
Q
5
⸮
6

2

Your array index should go from 0 to MAX_CHANNELS-1 not 1 to MAX_CHANNELS. if you overflow and write somewhere in memory then all bets are off --> rslights[5].type = 'Q' ; is not good

try with

for (int i = 0 ; ((i < MAX_CHANNELS) && (rslights[i].type != 'W')) ; ++i) {     
  Serial.println(rslights[i].type); 
  Serial.println(i);
}

and

rslights[0].type = 'X' ;
rslights[1].type = 'Y' ;
rslights[2].type = 'Z' ;
rslights[3].type = 'P' ;
rslights[4].type = 'Q' ;
3

ah thanks, I thought it would be 0 to 5. That does solve the problem, but now I am interested in why...

Even if I did overrun the array size, I would have expected it to stop after 5 - why does it not stop?

Clearly the second part of the condition would produce junk since the array index 5 does not exist, but its evaluation surely has to be either true or false?

4

When you write outside the array boundaries, you broke the rules so all bets are off - anything can happen.
That being said, you could imagine that the variable i is stored in a memory location right next to your rslights array such that rslights[5] is actually referencing the same (or partially the same) address space occupied by 'i'

5

Yeah it's fishy...

I'm ready to bet that if you run something that "feels' correct such as

const int NB = 5;  

struct RSLight {
  // design time values
  char type;   // L = monochrome light
  int  pin;   // raw pin number
};

RSLight rslights[NB];

void sendCHdata() {
  for (int i = 0 ; ((i <= NB) && (rslights[i].type != 'W') ) ; ++i) {
    if (i <= NB) {
      Serial.print(i);
      Serial.print(" is <= ");
      Serial.println(NB);
    }
  }
}

void setup() {
  Serial.begin(115200);
  rslights[0].type = 'X' ;
  sendCHdata();
}

void loop() {}

where we don't overflow anything it will still be buggy

the reason lies in accessing beyond the bounds of the structure (even just reading it) [nobbc]rslights[i].type [/nobbc] when i is 5. if you were to compile without the -Waggressive-loop-optimizations maybe you'll be safe.

--> don't mess around with memory that does not belong to you :slight_smile:

6

I understand changing memory that is out of bounds is bad.
As it turns out in my main project in which I ran into the problem, I was not attempting to set array element [5] So in fact the 'problem' manifested itself even without any invalid memory writes. Furthermore in my real data the stop value being checked for was in index 4, so the loop should have terminated before I even got out of bounds.

I also understand that attempting to read beyond the array bounds will return unpredictable values, just the same as reading uninitialized memory within the bounds of the array.

Evaluating garbage content in an expression should just mean that the result of that expression is unpredictable.
Since the outcome is irrelevant when i > 6, I still expect the for condition to become false.

As you say it may be something to do with the compiler options and some optimization.

My major error was thinking that array bounds were the upper index (as I am used to) and not the array count. Hopefully I have learnt that now.

Many thanks.

7

From a style point of view, i find having 2 conditions within a for loop declaration not preferred, personally i would make it a while() {} or even use break; (or return; if the whole thing is within a function) to see what the reason exiting the loop was.

8

À condition is à condition, so there is nothing wrong having it in a for loop. Just need to be sure one does not break the rules :slight_smile:

9

J-M-L:
À condition is à condition, so there is nothing wrong having it in a for loop.

no there isn't anything wrong in it, but a for loop is an assignment of a variable (including optional declaration) a condition and another assignment, all on one line. for overview purposes these things could also occupy a line each and if the condition is complex i do prefer that.

10

Ok - each coding style is respectable