Pointers and bit manipulation

[Haroon Shafiq]

hi,
I just want to know if the following statement conforms to standards?
Is it correct to write it this way?

/*CODE BEGINS*/
#define OFFSET = 0x1234
guchar* packet = getPacket();
guint32 payloadLength = 0;
[...]
payloadLength = (payloadLength << 16) | *((guint16 *)&packet[OFFSET]);
[...]
/*CODE ENDS*/

 

[Richard Bos]

I just want to know if the following statement conforms to standards?
Is it correct to write it this way?

/*CODE BEGINS*/
#define OFFSET = 0x1234
guchar* packet = getPacket();
guint32 payloadLength = 0;
[...]
payloadLength = (payloadLength << 16) | *((guint16 *)&packet[OFFSET]);
[...]
/*CODE ENDS*/

There's no way to tell if we don't know what a guchar, a guint32, a
guint16, and getPacket() are.

We might _guess_ that the gu in gufoo stand for Grich's unsigned (rather
than for, say, Graphical User, which would tell us nothing about the
underlying type), and that the types really _are_ all unsigned. We might
_guess_ that getPacket returns a pointer to a static or allocated
object, 0x1234 bytes into which are bytes which form a well-aligned
guint16 which never has a trap value. We might _guess_ that your first
[...] never sets payloadLength to a value which has important
information in its top 16 bits.

If _all_ these guesses are, by a stroke of luck, correct, then your code
is legal and does what we might _guess_ you meant it to do. If any of
these guesses is wrong, you may end up with the wrong information, and
if either of the first two is wrong, anything, from "apparently nothing"
to "my boss just fired me because my program crashed the customer's
computer" may happen.

Richard

 

[Keith Thompson]

I just want to know if the following statement conforms to standards?
Is it correct to write it this way?

/*CODE BEGINS*/
#define OFFSET = 0x1234
guchar* packet = getPacket();
guint32 payloadLength = 0;
[...]
payloadLength = (payloadLength << 16) | *((guint16 *)&packet[OFFSET]);
[...]
/*CODE ENDS*/

No. If you try compiling it, your compiler will tell you at least
some of the errors. (In particular, OFFSET expands to the two
tokens
= 0x1234
creating a syntax error.

Since you haven't shown us the definitions of guchar, guint16, or
guint32, it's hard to tell what other problems there might be.

One possible problem is that you convert the address of packet[OFFSET]
to a guint16*, and then dereference it; this might cause alignment
problems.

 

[Haroon Shafiq]

I just want to know if the following statement conforms to standards?
Is it correct to write it this way?

/*CODE BEGINS*/
#define OFFSET = 0x1234
guchar* packet = getPacket();
guint32 payloadLength = 0;
[...]
payloadLength = (payloadLength << 16) | *((guint16 *)&packet[OFFSET]);
[...]
/*CODE ENDS*/

No. If you try compiling it, your compiler will tell you at least
some of the errors. (In particular, OFFSET expands to the two
tokens
= 0x1234
creating a syntax error.

Since you haven't shown us the definitions of guchar, guint16, or
guint32, it's hard to tell what other problems there might be.

One possible problem is that you convert the address of packet[OFFSET]
to a guint16*, and then dereference it; this might cause alignment
problems.

okay, assume some thing like this
#define OFFSET = 12
char* packet = getPacket(); // getPacket() returns pointer a block of
some data.
unsigned int payloadLength = 0;

and payloadLength is calculated like this:

payloadLength = *((guint16 *)&packet[OFFSET - 4]);

payloadLength = (payloadLength << 16) | *((guint16 *)&packet[OFFSET]);

now what do you think of the last statement above? is it safe to code
this way or should I put value of *((guint16 *)&packet[OFFSET]) in a
temporary variable and then use it?

 

[Pedro Graca]

I just want to know if the following statement conforms to standards?
Is it correct to write it this way?

/*CODE BEGINS*/
#define OFFSET = 0x1234
guchar* packet = getPacket();
guint32 payloadLength = 0;
[...]
payloadLength = (payloadLength << 16) | *((guint16 *)&packet[OFFSET]);

Is the << operator defined for guint32 (whatever that is)?

What is the type of applying << to a guint32 and a int?
Maybe another guint32? If that is so, is the | operator defined for
guint32?

You're casting a pointer to guint32 to a pointer to guint16 and then
dereference this last pointer. Are you certain you can do that?

And, when the precompiler substitutes the #define's your last line will
turn to

payloadLength = (payloadLength << 16) | *((guint16 *)&packet[= 0x1234]);
^^^^^^^^
which is probably not what you want (and will not compile).

[...]
/*CODE ENDS*/

 

[slebetman]

I just want to know if the following statement conforms to standards?
Is it correct to write it this way?

/*CODE BEGINS*/
#define OFFSET = 0x1234
guchar* packet = getPacket();
guint32 payloadLength = 0;
[...]
payloadLength = (payloadLength << 16) | *((guint16 *)&packet[OFFSET]);
[...]
/*CODE ENDS*/

No. If you try compiling it, your compiler will tell you at least
some of the errors. (In particular, OFFSET expands to the two
tokens
= 0x1234
creating a syntax error.

Since you haven't shown us the definitions of guchar, guint16, or
guint32, it's hard to tell what other problems there might be.

One possible problem is that you convert the address of packet[OFFSET]
to a guint16*, and then dereference it; this might cause alignment
problems.

okay, assume some thing like this
#define OFFSET = 12
char* packet = getPacket(); // getPacket() returns pointer a block of
some data.
unsigned int payloadLength = 0;

and payloadLength is calculated like this:

payloadLength = *((guint16 *)&packet[OFFSET - 4]);

payloadLength = (payloadLength << 16) | *((guint16 *)&packet[OFFSET]);

now what do you think of the last statement above? is it safe to code
this way or should I put value of *((guint16 *)&packet[OFFSET]) in a
temporary variable and then use it?

Hmm.. it seems like you don't understand how #defines work.

#define OFFSET = 12

will generate in your code the following line after preprocessing:

... *((guint16 *)&packet[= 12]);

which is a syntax error since you're trying to index the packet array
with "= 12" instead of a simple integer. It should really be:

#define OFFSET 12

which will then generate the code you want after preprocessing:

... *((guint16 *)&packet[12]);

Assuming that the guint16 is a 16 bit integer then that code is not
portable on machines with strict alignment such as the ARM architecture
(StrongARM, ARM9, XScale etc..). The portable way to extract integers
from a byte stream is to use memcpy which takes care of properly
handling alignment.

 

[Haroon Shafiq]

okay, assume some thing like this
#define OFFSET = 12
char* packet = getPacket(); // getPacket() returns pointer a block of
some data.
unsigned int payloadLength = 0;

and payloadLength is calculated like this:

payloadLength = *((guint16 *)&packet[OFFSET - 4]);

payloadLength = (payloadLength << 16) | *((guint16 *)&packet[OFFSET]);

now what do you think of the last statement above? is it safe to code
this way or should I put value of *((guint16 *)&packet[OFFSET]) in a
temporary variable and then use it?

Hmm.. it seems like you don't understand how #defines work.

#define OFFSET = 12

yea its #define OFFSET 12,
i know i should avoid copy pasting.

... *((guint16 *)&packet[12]);

its `int` not `guint16` or `guint32`, copy pastes again.

 

[stathis gotsis]

okay, assume some thing like this
#define OFFSET = 12
char* packet = getPacket(); // getPacket() returns pointer a block of
some data.
unsigned int payloadLength = 0;

and payloadLength is calculated like this:

payloadLength = *((guint16 *)&packet[OFFSET - 4]);

payloadLength = (payloadLength << 16) | *((guint16 *)&packet[OFFSET]);

now what do you think of the last statement above? is it safe to code
this way or should I put value of *((guint16 *)&packet[OFFSET]) in a
temporary variable and then use it?

Hmm.. it seems like you don't understand how #defines work.

#define OFFSET = 12

yea its #define OFFSET 12,
i know i should avoid copy pasting.

... *((guint16 *)&packet[12]);

its `int` not `guint16` or `guint32`, copy pastes again.

Still, you need to consider alignment issues and use memcpy() to copy
sizeof(int) bytes from packet[] into an int. Then, you can use that int in
your expression.