Operation maybe undefined in macro

Question

Guys i have next two pieces of code which extracts 32bit variable from 8bit 1) First is that:

#include <stdio.h>
#include <string.h>

int main()
{
    unsigned char buffer[] = {0xaa, 0xbb, 0xcc, 0xdd, 0xee};
    unsigned char *buff = buffer;
    unsigned int  result;
    result = *buff++;
    result += *buff++ <<8;
    result += *buff++ << 16;
    result += *buff++ <<24;

    printf("result = 0x%x, *buffer = 0x%x.", result, *buff);

    return 0;
}

Without any warning, but it looks a little lame....

2) In second we have macro instead of those ugly 4 lines:

#include <stdio.h>
#include <string.h>

#define to32(buffer) ((unsigned int)*buffer++ | *buffer++ << 8 | *buffer++ << 16 | *buffer++ << 24)

int main()
{
    unsigned char buffer[] = {0xaa, 0xbb, 0xcc, 0xdd, 0xee};
    unsigned char *buff = buffer;
    unsigned int  result = to32(buff);

    printf("result = 0x%x, *buffer = 0x%x.", result, *buff);

    return 0;
}

And it leaves next warning:

main.cpp: In function 'int main()':

main.cpp:4:99: warning: operation on 'buff' may be undefined [-Wsequence-point]

 #define to32(buffer) ((unsigned int)*buffer++ | *buffer++ << 8 | *buffer++ << 16 | *buffer++ << 24)

And i'm little confused what's exactly GCC found as undefined behavior. Is it that all shifts in one line and i sum it?

I'll look into it. Basically i'm confused because both parts of code are almost same but for some reason when you write it like that macro it is compiled with warning — Douman, Mar 30 '14 at 16:02

Gábor Buella · Accepted Answer · 2014-03-30T16:14:13.223

2

EDIT A solution is:

static inline unsigned to32(const unsigned char *buffer)
{
    return buffer[0] | buffer[1] << 8 | buffer[1] << 16 | buffer[3] << 24;
}

If you really don't like functions, you can do the same with a macro...

It is about those sequence-points the compiler warns you about.

The order is not defined here:

  to32(buffer) ((unsigned int)*buffer++ | *buffer++ << 8 | *buffer++ << 16 | *buffer++ << 24)
  // will be
     buffer = *buffer++;
     buffer |= *buffer++ << 8;
      ....

   // or
     buffer = *buffer++ << 8;
     buffer |= *buffer++;
    ...

In case of || and && there is a sequence point at the operator, and the operations have an order, going left to right. But not at | and & The order is not defined, the compiler can do it any order.

edited Mar 30 '14 at 16:14

answered Mar 30 '14 at 16:05

Gábor Buella

1,840
14
22

And same will be for '+' i guess? That's kinda sad :( I would love to have one macro for that. Actually i find it's kinda strange that order isn't strict for those operators – Douman Mar 30 '14 at 16:08
2

@Douman You can write an equivalent expression that doesn't use the `++` operator, for which the sequence points won't matter. – augurar Mar 30 '14 at 16:09
Yes, you can just index your `buffer` – Gábor Buella Mar 30 '14 at 16:14
@auguar, But i need it to get next bytes from *buff and i use it to make it more compact. I also need it as i'm planing to read from such buff several times and i guess it's not the best to make +4 after each extracting :) I guess i would better to write usual function instead of macro. Anyway thanks for explanation! – Douman Mar 30 '14 at 16:15

score 0 · Answer 2 · answered Mar 30 '14 at 16:33

After some suggestions from Buella Gabor and auguar it came to me that i can do it like that. And it seems to be right

#define to32(buffer) ((unsigned int)*buffer | *(buffer+1) << 8 | *(buffer+2) << 16 | *(buffer+3) << 24)

Though it's not perfect for my needs :(

Operation maybe undefined in macro

2 Answers2