Stephan GM Stephan GM - 4 months ago 28
C Question

Array values reset in loop

So I am using

poll()
to read a couple gpio pins. I then compare previously read values to the newly read ones to see if they have changed. The reading of the values works just fine. My problem seems to be in the loop. This can be seen from the output where it looks like
buffers
is reset at the beginning of the loop. Why is this happening?

Note: In case anyone is wondering why I don't just use
poll()
as an interrupt with a delay of
-1
, it is because of a hardware issue that makes it unsupported.

Code



static const int num_buttons = 2;
char buffers[num_buttons][2];
char prev_buffers[num_buttons][2];

for (;;) {

LOGD("at top: prev:%d%d buff:%d%d",
atoi(prev_buffers[0]), atoi(prev_buffers[1]),
atoi(buffers[0]), atoi(buffers[1]));

poll(pfd, num_buttons, 1);

for (i = 0; i < num_buttons; i++) {
if ((pfd[i].revents & POLLIN)) {

/* copy current values to compare to next to detected change */
strcpy(prev_buffers[i], buffers[i]);
LOGD("in loop: prev:%d%d buff:%d%d",
atoi(prev_buffers[0]), atoi(prev_buffers[1]),
atoi(buffers[0]), atoi(buffers[1]));

/* read new values */
lseek(fds[i], 0, SEEK_SET);
read(fds[i], buffers[i], sizeof buffers[i]);

/* compare new to previous */
if (atoi(prev_buffers[i]) != atoi(buffers[i])) {
// LOGD("change detected");
}
}
}
}


Output



at top: prev:00 buff:01
in loop: prev:01 buff:00
in loop: prev:00 buff:00
at top: prev:00 buff:01
in loop: prev:01 buff:00
in loop: prev:00 buff:00

Answer

This line seems risky:

strcpy(prev_buffers[i], buffers[i]);

prev_buffers[i] is only 2 bytes long. If buffers[i] has more than 1 character, you have a buffer overflow and invoke undefined behavior.

Furthermore, you must initialize buffers, they currently start with random junk, not single byte character strings: using strcpy to save the previous values invokes undefined behavior. Using memcpy() to save whatever previous contents is less risky as it will copy just 2 bytes, instead of scanning the buffers for a null terminator, potentially invoking undefined behavior both when reading past the end of buffers[i] and when writing past the end of prev_buffers[i].

Are you sure that read(fds[i], buffers[i], sizeof buffers[i]); reads an ASCII digit and a null terminator? If not, the code is incorrect.

Post a complete function that compiles, there might be more problems.

Comments