sizeof and structure

Discussion in 'C Programming' started by Ravishankar S, Jan 9, 2008.

  1. "Roman Mashak" <> wrote in message
    news:fm24jp$2t4m$...

    > enum usb_offset {
    > usb_idx_function = 0,
    > usb_idx_size = usb_idx_function + sizeof(unsigned char),
    > usb_idx_i2c_addr = usb_idx_size + sizeof(unsigned char),
    > usb_idx_i2c_subaddr = usb_idx_i2c_addr + sizeof(unsigned char),
    > usb_idx_i2c_datasize = usb_idx_i2c_subaddr + sizeof(unsigned char),
    > usb_idx_i2c_data = usb_idx_i2c_datasize + sizeof(unsigned char)
    > };
    >

    break;
    > ...
    > }
    >
    > At this point I expect to have 'usb_idx_i2c_addr' equal to 'data[3]', but


    'usb_idx_i2c_addr' will be equal to 3 (the index) , not data[3]. enumeration
    values _are_ the offsets into
    the data[] array.
     
    Ravishankar S, Jan 9, 2008
    #1
    1. Advertising

  2. "Ravishankar S" <> wrote in message
    news:fm2752$jup$...
    >
    > "Roman Mashak" <> wrote in message
    > news:fm24jp$2t4m$...
    >
    > > enum usb_offset {
    > > usb_idx_function = 0,
    > > usb_idx_size = usb_idx_function + sizeof(unsigned char),
    > > usb_idx_i2c_addr = usb_idx_size + sizeof(unsigned char),
    > > usb_idx_i2c_subaddr = usb_idx_i2c_addr + sizeof(unsigned char),
    > > usb_idx_i2c_datasize = usb_idx_i2c_subaddr + sizeof(unsigned char),
    > > usb_idx_i2c_data = usb_idx_i2c_datasize + sizeof(unsigned char)
    > > };
    > >

    > break;
    > > ...
    > > }
    > >
    > > At this point I expect to have 'usb_idx_i2c_addr' equal to 'data[3]',

    but
    > values _are_ the offsets into
    > the data[] array.


    Infact 'usb_idx_i2c_addr' is not 3 , but 2.
    usb_idx_function = 0;
    usb_idx_size = usb_idx_function + sizeof(unsigned char) = 0 + 1 = 1
    usb_idx_i2c_addr = usb_idx_size + sizeof(unsigned char) = 1 + 1 = 2
     
    Ravishankar S, Jan 9, 2008
    #2
    1. Advertising

  3. Ravishankar S

    fnegroni Guest

    Can I ask a silly question?
    Why are you using artificially manufactured structs to parse a byte
    array?
    Is it to make access to the byte array's sections more readable?
    Why not just use macros?

    I am a newbie so I am just curious to know.
     
    fnegroni, Jan 9, 2008
    #3
  4. "fnegroni" <> wrote in message
    news:...
    > Can I ask a silly question?
    > Why are you using artificially manufactured structs to parse a byte
    > array?
    > Is it to make access to the byte array's sections more readable?
    > Why not just use macros?
    >
    > I am a newbie so I am just curious to know.

    I suppose its for readability only. comprehension is better when structs are
    used.
     
    Ravishankar S, Jan 9, 2008
    #4
  5. "Ravishankar S" <> writes:

    > "fnegroni" <> wrote in message
    > news:...
    >> Can I ask a silly question?
    >> Why are you using artificially manufactured structs to parse a byte
    >> array?
    >> Is it to make access to the byte array's sections more readable?
    >> Why not just use macros?
    >>
    >> I am a newbie so I am just curious to know.

    > I suppose its for readability only. comprehension is better when structs are
    > used.


    As someone who suggested using offsets + access functions I have to
    say that it depends on how well the access functions are written. The
    code can be made very readable but, more importantly, can be made more
    portable (note that some byte fields are really parts of larger
    types -- size_lo size_hi).

    What is odd is to see both approaches "half" used.

    --
    Ben.
     
    Ben Bacarisse, Jan 9, 2008
    #5
  6. "Roman Mashak" <> writes:

    > I already asked about my problem and received several valuable advices which
    > I have followed to. Anyway I should explain briefly.
    > In the program I'm working on I receive a data stream, having structure as
    > follows:
    >
    > | common_header | I2C_header | data ... |


    Just to be clear... you call it a "common header". Do you sometimes
    need to process messages like this:

    | common_header | data ... |
    or
    | common_header | some_other_header | data ... |
    ?
    >
    >
    > My purpose is to parse the 'common_header', and read the data following it.
    >
    > This is my code snippet:
    >
    > #define MSG_SIZE 1000
    >
    > enum ctrl_cmds {
    > CMD_ECHO = 0x0,
    > CMD_I2C_WRITE,
    > CMD_I2C_READ,
    > CMD_I2C_BURST_WRITE
    > };
    >
    > struct __attribute__ ((packed)) USB_iobuf_common_header_s {
    > unsigned char function;
    > unsigned char size_lo;
    > unsigned char size_hi;
    > };
    >
    > typedef struct USB_iobuf_common_header_s USB_iobuf_common_header;
    >
    > struct __attribute__ ((packed)) USB_iobuf_i2c_header_s {
    > unsigned char addr;
    > unsigned char subaddr_lo;
    > unsigned char subaddr_hi;
    > unsigned char datasize_lo;
    > unsigned char datasize_hi;
    > };
    >
    > typedef struct USB_iobuf_i2c_header_s USB_iobuf_i2c_header;
    >
    > enum usb_offset {


    When I posted to suggest using an offset enum like this, it was as an
    alternative to non-portable packed structs. Not in addition to them!
    This just gives you two things to maintain.

    > usb_idx_function = 0,
    > usb_idx_size = usb_idx_function + sizeof(unsigned char),
    > usb_idx_i2c_addr = usb_idx_size + sizeof(unsigned char),


    I thought I wrote 'usb_idx_size + sizeof(uint16_t),'. I certainly
    intended you to document the offsets by using the sizes of the data in
    the header and the function is followed by two bytes, not one. Of
    course, you can have an index for usb_idx_size_lo and usb_idx_size_hi
    if you want and then have the index of addr be that for size_hi + 1.

    > usb_idx_i2c_subaddr = usb_idx_i2c_addr + sizeof(unsigned char),
    > usb_idx_i2c_datasize = usb_idx_i2c_subaddr + sizeof(unsigned char),
    > usb_idx_i2c_data = usb_idx_i2c_datasize + sizeof(unsigned char)
    > };
    >
    > char data[MSG_SIZE];


    unsigned char is usually safer.

    --
    Ben.
     
    Ben Bacarisse, Jan 9, 2008
    #6
  7. On Jan 9, 9:36 pm, "Roman Mashak" <> wrote:
    > Hello,
    >
    > I already asked about my problem and received several valuable advices which
    > I have followed to. Anyway I should explain briefly.
    > In the program I'm working on I receive a data stream, having structure as
    > follows:
    >
    >       |  common_header | I2C_header | data ... |
    >
    > My purpose is to parse the 'common_header', and read the data following it..
    >
    > This is my code snippet:
    >
    > #define MSG_SIZE 1000
    >
    > enum ctrl_cmds {
    >     CMD_ECHO = 0x0,
    >     CMD_I2C_WRITE,
    >     CMD_I2C_READ,
    >     CMD_I2C_BURST_WRITE
    >
    > };
    >
    > struct __attribute__ ((packed)) USB_iobuf_common_header_s {
    >     unsigned char function;
    >     unsigned char size_lo;
    >     unsigned char size_hi;
    >
    > };
    >
    > typedef struct USB_iobuf_common_header_s USB_iobuf_common_header;
    >
    > struct __attribute__ ((packed)) USB_iobuf_i2c_header_s {
    >     unsigned char addr;
    >     unsigned char subaddr_lo;
    >     unsigned char subaddr_hi;
    >     unsigned char datasize_lo;
    >     unsigned char datasize_hi;
    >
    > };
    >
    > typedef struct USB_iobuf_i2c_header_s USB_iobuf_i2c_header;
    >
    > enum usb_offset {
    >      usb_idx_function = 0,
    >      usb_idx_size  = usb_idx_function  + sizeof(unsigned char),
    >      usb_idx_i2c_addr = usb_idx_size   + sizeof(unsigned char),
    >      usb_idx_i2c_subaddr = usb_idx_i2c_addr  + sizeof(unsigned char),
    >      usb_idx_i2c_datasize = usb_idx_i2c_subaddr + sizeof(unsigned char),
    >      usb_idx_i2c_data     = usb_idx_i2c_datasize + sizeof(unsigned char)
    >
    > };
    >
    > char data[MSG_SIZE];
    > char reg;
    > ...
    >
    > CDC_Read(..., data, MSG_SIZE);
    >
    > USB_iobuf_common_header *hdr = (USB_iobuf_common_header *)data;
    > switch (hdr->function) {
    >     case CMD_I2C_READ:
    >         AT91F_TWI_ReadSingleIadr(AT91C_BASE_TWI, (usb_idx_i2c_addr << 16),
    > data[4], AT91C_TWI_IADRSZ_1_BYTE, &reg);
    >          ...
    >         break;
    >         ...
    >
    > }
    >
    > At this point I expect to have 'usb_idx_i2c_addr' equal to 'data[3]', but
    > it's always 0, though the other end is sending 0xC0. If I use 'data[3]' or
    > 'data[4]' - everything works perfectly, but this is ugly hack.
    > I'm stuck. What's the problem with that?
    >
    > The part of problem problem was with alignment of structure, I found out how
    > to disable padding and I also added the offsets (via 'enum') to be more
    > readable.
    >
    > Thanks.
    >
    > With best regards, Roman Mashak.  E-mail:


    Roman,

    A better approach would be for you to create instances of the
    structure and populate them as you read the stream.
    Since u have declared the structs are 'packed' they are contigeous and
    hence no padding of any sort. So in your case if you want to read the
    common header

    USB_iobuf_i2c_header header;

    read(filehandle, &header, sizeof(USB_iobuf_i2c_header));

    At this point the file pointer will be pointing to I2C_header and you
    could use the same approach to populate the appropriate buffers.

    This may help you alleviate some issues.

    Suresh Shenoy
     
    suresh shenoy, Jan 9, 2008
    #7
  8. Ravishankar S

    Roman Mashak Guest

    Hello,

    I already asked about my problem and received several valuable advices which
    I have followed to. Anyway I should explain briefly.
    In the program I'm working on I receive a data stream, having structure as
    follows:

    | common_header | I2C_header | data ... |


    My purpose is to parse the 'common_header', and read the data following it.

    This is my code snippet:

    #define MSG_SIZE 1000

    enum ctrl_cmds {
    CMD_ECHO = 0x0,
    CMD_I2C_WRITE,
    CMD_I2C_READ,
    CMD_I2C_BURST_WRITE
    };

    struct __attribute__ ((packed)) USB_iobuf_common_header_s {
    unsigned char function;
    unsigned char size_lo;
    unsigned char size_hi;
    };

    typedef struct USB_iobuf_common_header_s USB_iobuf_common_header;

    struct __attribute__ ((packed)) USB_iobuf_i2c_header_s {
    unsigned char addr;
    unsigned char subaddr_lo;
    unsigned char subaddr_hi;
    unsigned char datasize_lo;
    unsigned char datasize_hi;
    };

    typedef struct USB_iobuf_i2c_header_s USB_iobuf_i2c_header;

    enum usb_offset {
    usb_idx_function = 0,
    usb_idx_size = usb_idx_function + sizeof(unsigned char),
    usb_idx_i2c_addr = usb_idx_size + sizeof(unsigned char),
    usb_idx_i2c_subaddr = usb_idx_i2c_addr + sizeof(unsigned char),
    usb_idx_i2c_datasize = usb_idx_i2c_subaddr + sizeof(unsigned char),
    usb_idx_i2c_data = usb_idx_i2c_datasize + sizeof(unsigned char)
    };

    char data[MSG_SIZE];
    char reg;
    ....

    CDC_Read(..., data, MSG_SIZE);

    USB_iobuf_common_header *hdr = (USB_iobuf_common_header *)data;
    switch (hdr->function) {
    case CMD_I2C_READ:
    AT91F_TWI_ReadSingleIadr(AT91C_BASE_TWI, (usb_idx_i2c_addr << 16),
    data[4], AT91C_TWI_IADRSZ_1_BYTE, &reg);
    ...
    break;
    ...
    }

    At this point I expect to have 'usb_idx_i2c_addr' equal to 'data[3]', but
    it's always 0, though the other end is sending 0xC0. If I use 'data[3]' or
    'data[4]' - everything works perfectly, but this is ugly hack.
    I'm stuck. What's the problem with that?

    The part of problem problem was with alignment of structure, I found out how
    to disable padding and I also added the offsets (via 'enum') to be more
    readable.

    Thanks.


    With best regards, Roman Mashak. E-mail:
     
    Roman Mashak, Jan 10, 2008
    #8
  9. "Roman Mashak" <> writes:

    > Hello, Ben!
    > You wrote on Wed, 09 Jan 2008 11:35:25 +0000:
    >
    > ??|>> common_header | I2C_header | data ... |
    >
    > BB> Just to be clear... you call it a "common header". Do you sometimes
    > BB> need to process messages like this:
    >
    > ??|> common_header | data ... |
    > BB> or
    > ??|> common_header | some_other_header | data ... |
    > BB> ?
    >
    > Probably never. The format of message is defined and fixed.


    Then you could consider putting both headers in one struct and you
    will (most likely) not have to do any system-specif packing magic --
    the header will consist of 8 chars.

    If, for some reason you all need indexes into a char array, you can
    use the offsetof macro to get them.

    <snip>
    > Making indexes readable is suffice, then the offsets really should look like
    > this:
    >
    > enum usb_offset {
    > usb_idx_function = 0,
    > usb_idx_size = usb_idx_function + sizeof(unsigned char),
    > usb_idx_i2c_addr = usb_idx_size + sizeof(unsigned short),
    > usb_idx_i2c_subaddr = usb_idx_i2c_addr + sizeof(unsigned char),
    > usb_idx_i2c_datasize = usb_idx_i2c_subaddr + sizeof(unsigned short),
    > usb_idx_i2c_data = usb_idx_i2c_datasize + sizeof(unsigned short)
    > };
    >
    > Am I correct?


    Yes, but you might have to check short is the size you want. That is
    why I suggested uint16_t.

    Having indexes without access functions seems pointless to me. If you
    want to have both the struct and the index enum, why not use offsetof?
    It would be more readable than my method.

    --
    Ben.
     
    Ben Bacarisse, Jan 10, 2008
    #9
  10. Ravishankar S

    Roman Mashak Guest

    Hello, Ben!
    You wrote on Wed, 09 Jan 2008 11:35:25 +0000:

    ??|>> common_header | I2C_header | data ... |

    BB> Just to be clear... you call it a "common header". Do you sometimes
    BB> need to process messages like this:

    ??|> common_header | data ... |
    BB> or
    ??|> common_header | some_other_header | data ... |
    BB> ?

    Probably never. The format of message is defined and fixed.

    ??>>
    BB> When I posted to suggest using an offset enum like this, it was as an
    BB> alternative to non-portable packed structs. Not in addition to them!
    BB> This just gives you two things to maintain.

    ??>> usb_idx_function = 0,
    ??>> usb_idx_size = usb_idx_function + sizeof(unsigned char),
    ??>> usb_idx_i2c_addr = usb_idx_size + sizeof(unsigned char),

    BB> I thought I wrote 'usb_idx_size + sizeof(uint16_t),'. I certainly
    BB> intended you to document the offsets by using the sizes of the data in
    BB> the header and the function is followed by two bytes, not one. Of
    BB> course, you can have an index for usb_idx_size_lo and usb_idx_size_hi
    BB> if you want and then have the index of addr be that for size_hi + 1.

    Making indexes readable is suffice, then the offsets really should look like
    this:

    enum usb_offset {
    usb_idx_function = 0,
    usb_idx_size = usb_idx_function + sizeof(unsigned char),
    usb_idx_i2c_addr = usb_idx_size + sizeof(unsigned short),
    usb_idx_i2c_subaddr = usb_idx_i2c_addr + sizeof(unsigned char),
    usb_idx_i2c_datasize = usb_idx_i2c_subaddr + sizeof(unsigned short),
    usb_idx_i2c_data = usb_idx_i2c_datasize + sizeof(unsigned short)
    };

    Am I correct?

    With best regards, Roman Mashak. E-mail:
     
    Roman Mashak, Jan 10, 2008
    #10
  11. Ravishankar S

    Roman Mashak Guest

    Hello, Ben!
    You wrote on Thu, 10 Jan 2008 11:42:10 +0000:

    [skip]
    BB> Then you could consider putting both headers in one struct and you will
    BB> (most likely) not have to do any system-specif packing magic -- the
    BB> header will consist of 8 chars.

    BB> Yes, but you might have to check short is the size you want. That is
    BB> why I suggested uint16_t.

    BB> Having indexes without access functions seems pointless to me. If you
    BB> want to have both the struct and the index enum, why not use offsetof?
    BB> It would be more readable than my method.

    Oh, great idea. Why didn't I consider 'offsetof' macro.
    Thanks!

    With best regards, Roman Mashak. E-mail:
     
    Roman Mashak, Jan 11, 2008
    #11
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Derek
    Replies:
    7
    Views:
    24,342
    Ron Natalie
    Oct 14, 2004
  2. Trevor

    sizeof(str) or sizeof(str) - 1 ?

    Trevor, Apr 3, 2004, in forum: C Programming
    Replies:
    9
    Views:
    636
    CBFalconer
    Apr 10, 2004
  3. Vinu
    Replies:
    13
    Views:
    1,437
    Lawrence Kirby
    May 12, 2005
  4. Alex Vinokur
    Replies:
    7
    Views:
    499
    Clark S. Cox III
    Aug 14, 2006
  5. Alex Vinokur

    sizeof (size_t) and sizeof (pointer)

    Alex Vinokur, Nov 12, 2007, in forum: C++
    Replies:
    19
    Views:
    793
    Ben Rudiak-Gould
    Nov 30, 2007
Loading...

Share This Page