Pointers and bit manipulation

Discussion in 'C Programming' started by Haroon Shafiq, Feb 16, 2006.

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

    #define OFFSET = 0x1234
    guchar* packet = getPacket();
    guint32 payloadLength = 0;
    payloadLength = (payloadLength << 16) | *((guint16 *)&packet[OFFSET]);
    /*CODE ENDS*/
    Haroon Shafiq, Feb 16, 2006
    1. Advertisements

  2. Haroon Shafiq

    Richard Bos Guest

    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 Bos, Feb 16, 2006
    1. Advertisements

  3. No. If you try compiling it, your compiler will tell you at least
    some of the errors. (In particular, OFFSET expands to the two
    = 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
    Keith Thompson, Feb 16, 2006
  4. 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?
    Haroon Shafiq, Feb 16, 2006
  5. Haroon Shafiq

    Pedro Graca Guest

    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

    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).

    Pedro Graca, Feb 16, 2006
  6. Haroon Shafiq

    slebetman Guest

    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.
    slebetman, Feb 16, 2006
  7. yea its #define OFFSET 12,
    i know i should avoid copy pasting.

    its `int` not `guint16` or `guint32`, copy pastes again.
    Haroon Shafiq, Feb 16, 2006
  8. 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.
    stathis gotsis, Feb 16, 2006
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.