Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > sizeof and structure

Reply
Thread Tools

sizeof and structure

 
 
Ravishankar S
Guest
Posts: n/a
 
      01-09-2008

"Roman Mashak" <(E-Mail Removed)> wrote in message
news:fm24jp$2t4m$(E-Mail Removed)...

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


 
Reply With Quote
 
 
 
 
Ravishankar S
Guest
Posts: n/a
 
      01-09-2008
"Ravishankar S" <(E-Mail Removed)> wrote in message
news:fm2752$jup$(E-Mail Removed)...
>
> "Roman Mashak" <(E-Mail Removed)> wrote in message
> news:fm24jp$2t4m$(E-Mail Removed)...
>
> > 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




 
Reply With Quote
 
 
 
 
fnegroni
Guest
Posts: n/a
 
      01-09-2008
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.
 
Reply With Quote
 
Ravishankar S
Guest
Posts: n/a
 
      01-09-2008
"fnegroni" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed)...
> 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.


 
Reply With Quote
 
Ben Bacarisse
Guest
Posts: n/a
 
      01-09-2008
"Ravishankar S" <(E-Mail Removed)> writes:

> "fnegroni" <(E-Mail Removed)> wrote in message
> news:(E-Mail Removed)...
>> 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.
 
Reply With Quote
 
Ben Bacarisse
Guest
Posts: n/a
 
      01-09-2008
"Roman Mashak" <(E-Mail Removed)> 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.
 
Reply With Quote
 
suresh shenoy
Guest
Posts: n/a
 
      01-09-2008
On Jan 9, 9:36*pm, "Roman Mashak" <(E-Mail Removed)> 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: (E-Mail Removed)


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
 
Reply With Quote
 
Roman Mashak
Guest
Posts: n/a
 
      01-10-2008
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: http://www.velocityreviews.com/forums/(E-Mail Removed)


 
Reply With Quote
 
Ben Bacarisse
Guest
Posts: n/a
 
      01-10-2008
"Roman Mashak" <(E-Mail Removed)> 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.
 
Reply With Quote
 
Roman Mashak
Guest
Posts: n/a
 
      01-10-2008
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: (E-Mail Removed)


 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
sizeof (size_t) and sizeof (pointer) Alex Vinokur C++ 19 11-30-2007 11:11 PM
sizeof(EmptyStruct) in C and C++ (was: Base {}; sizeof(Base) == 1?) Alex Vinokur C Programming 7 08-14-2006 04:57 PM
#define ARR_SIZE sizeof(arr)/sizeof(arr[0]) Vinu C Programming 13 05-12-2005 06:00 PM
sizeof(enum) == sizeof(int) ??? Derek C++ 7 10-14-2004 05:11 PM
sizeof(str) or sizeof(str) - 1 ? Trevor C Programming 9 04-10-2004 05:07 PM



Advertisments