if statement doesn't seem to ever fail

Question

I have a loop which goes through an image stored in a buffer, grabs 1024 bytes at a time, and sends this kilobyte off as a UDP packet. Below is the relevant section of code.

Why doesn't my second if statement, if (buf.bytesused - curr*1024 > 1024) fail when curr*1024 gets larger than buf.bytesused? I'm getting stuck in an infinite loop, which doesn't end until memcpy finally throws a seg fault. My buf.bytesused is around 900,000, but instead of looping about 900 times, it just keeps going, even when buf.bytesused - curr*1024 gives a negative result, which is certainly less than 1024.

In case that wasn't clear: When curr is small, buf.bytesused - curr*1024 > 1024 is satisfied like I'd expect, and this second if-block is run. However, when curr get large (even so large that buf.bytesused - curr*1024 is negative), the second if-block is still being run, instead of the third block, after the else.

buf.bytesused is an unsigned 32 bit int.

  #define FRAME_START 0x0
  #define FRAME_END   0xF
  #define FRAME_MID   0xA

  uint_32 buf.bytesused = 921600;
  char frameInfo = FRAME_START;
  char frameNum = i;
  int curr = 0;

  while (frameInfo != FRAME_END) {
    if (curr == 0) {
      frameInfo = FRAME_START;
      memcpy(&frameInfo, &udpBuf[0], 1);
      memcpy(&frameNum, &udpBuf[1], 1);
      memcpy(buffers[buf.index].start + curr*1024, udpBuf + 2, 1024);
    }else if (buf.bytesused - curr*1024 > 1024) {
      frameInfo = FRAME_MID;
      memcpy(&frameInfo, &udpBuf[0], 1);
      memcpy(&frameNum, &udpBuf[1], 1);
      memcpy(buffers[buf.index].start + curr*1024, udpBuf + 2, 1024);
    } else {
      frameInfo = FRAME_END;
      memcpy(&frameInfo, &udpBuf[0], 1);
      memcpy(&frameNum, &udpBuf[1], 1);
      memcpy(buffers[buf.index].start + curr*1024, udpBuf + 2, (buf.bytesused % 1024));
    }

    if (sendto(s, udpBuf, BUFLEN, 0, &si_other, slen)==-1)
      diep("sendto()");
    curr++;
  }

As an aside, could someone tell me what I'm doing wrong with memcpy? It doesn't copy the values of my two chars to my buffer.

Are you sure the issue is with that if condition? It looks like you are overwriting `frameInfo` with whatever is in `udpBuf[0]`, meaning that it is highly unlikely to ever stay as `FRAME_END`. edit: oh, are those supposed to be the other way around? You might want to look at the parameter list for `memcpy` and see if you are using it correctly — Red Alert, Nov 28 '13 at 02:56
Is `buf.bytesused` a `size_t` or other unsigned integer type? If so, the subtraction will be done using unsigned arithmetic, and won't ever go negative. — Jonathan Leffler, Nov 28 '13 at 02:57
Sorry if I wasn't clear in the question. I hope this is more clear: When `curr` is small, `buf.bytesused - curr*1024 > 1024` is satisfied like I'd expect, and this second if-block is run. However, when `curr` get large (even so large that `buf.bytesused - curr*1024` is negative), the second if-block is still being run, instead of the third block, after the `else`. I don't understand why, though in my experience it usually means I'm overlooking something very basic. — WhiteHotLoveTiger, Nov 28 '13 at 02:58
Please update the question to clarify it, rather than adding a comment. You could provide some examples; you could also provide definitions (type information) for the variables in the question. — Jonathan Leffler, Nov 28 '13 at 03:00
Post the rest of your code so we can see how you've implemented the buffers. No one knows what `buf.bytesused` means. — Red Alert, Nov 28 '13 at 03:02
Sorry guys, I should have known better. I will update this in a few minutes. — WhiteHotLoveTiger, Nov 28 '13 at 03:05
I'm working on making a minimal working example. In the meantime, the entire code is here if anyone wants to take a look: http://pastebin.com/MSvpUxLQ — WhiteHotLoveTiger, Nov 28 '13 at 03:13
@WhiteHotLoveTiger: Edited the solution. Please see, if it can help you. — doptimusprime, Nov 28 '13 at 03:20
I fixed all the memcpy mistakes I had (source & destination were reversed -- and I had even checked them in the man pages, though I must have been half asleep when I did...). After making the proper memcpy calls, everything is suddenly working. When I print out the values of `buf.bytesused - curr*1024`, it goes nice and orderly, just as I would expect it to. I guess I must have been overwriting something with the improper call. In any event, the buf.bytesused value is indeed an unsigned 32 bit int. — WhiteHotLoveTiger, Nov 28 '13 at 03:39
@WhiteHotLoveTiger See my previous comment. It had nothing to do with your if statement, the problem is you were overwriting `frameInfo` with memcopy. In the future, make sure you've actually verified that your program is stuck in an if statement before you claim that it is. — Red Alert, Nov 28 '13 at 04:17

doptimusprime · Answer 1 · 2013-11-28T03:19:41.700

4

if (buf.bytesused - curr*1024 > 1024) fail when curr*1024 gets larger than buf.bytesused

It may be problem when buf.bytesused is unsigned.

If it is negative and signed, then the above comparison should fail.

To solve, you can use simple maths too.

if (buf.bytesused > curr*1024 + 1024)

or

if (buf.bytesused > (curr + 1)*1024)

These conditions will not cause problem due to sign unless curr+1 is negative.

I think this should solve your problem.

edited Nov 28 '13 at 03:19

answered Nov 28 '13 at 02:57

doptimusprime

9,115
6
52
90

1

This is highly unlikely to be the issue. Unless buf.bytesused is less than 1024, this condition will eventually fail EVEN if bytesused is unsigned. I don't think this is what's wrong. – Red Alert Nov 28 '13 at 03:05
@RedAlert - The unsigned result could be > 1024 right off the bat. – Hot Licks Nov 28 '13 at 03:27
@Hot Licks - there is no "result" in his code, but if you are talking about bytesused, then it should work fine if it starts > 1024. As curr increments, eventually the expression will evaluate to something between 0 and 1024, which will cause the condition to fail. – Red Alert Nov 28 '13 at 03:35
Sorry I didn't have this information from the beginning, but I've updated the question to indicate that buf.bytesused is an unsigned 32 bit int, with an initial value of 921600. – WhiteHotLoveTiger Nov 28 '13 at 03:46
@RedAlert - I was talking about the result of the (possibly unsigned) subtraction. All it takes is for curr to be 1 and bytesused to be < 1024. – Hot Licks Nov 28 '13 at 03:51
@Hot Licks - Is that not exactly what I said initially? – Red Alert Nov 28 '13 at 04:15
@RedAlert - No, you said that unsigned arithmetic is unlikely to be the issue, when it's quite likely. – Hot Licks Nov 28 '13 at 04:24
@Hot Licks - I opened with that potential race condition, and it IS unlikely. The actual value is orders of magnitude bigger than 1024, so there's no chance of this ever being a problem. The checkmark & upvotes on this answer will only confuse people, since it has nothing to do with OP's issue. – Red Alert Nov 28 '13 at 04:28
@RedAlert - Who mentioned a race condition?? – Hot Licks Nov 28 '13 at 13:11

score 3 · Accepted Answer · answered Nov 28 '13 at 04:33

In case people are wondering, the actual problem is here:

frameInfo = FRAME_END;
memcpy(&frameInfo, &udpBuf[0], 1);

frameInfo is overwritten after it is set to FRAME_END, so while (frameInfo != FRAME_END) will never evaluate to false. The fact that this caused the program to enter an infinite loop lead the OP to mistakenly believe that his if statement was wrong. It's always good practice to use a debugger instead of assuming you know what the problem is!

if statement doesn't seem to ever fail

2 Answers2