sizeof and structure

[Roman Mashak]

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_W RITE
};

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

typedef struct USB_iobuf_commo n_header_s USB_iobuf_commo n_header;

struct __attribute__ ((packed)) USB_iobuf_i2c_h eader_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_h eader_s USB_iobuf_i2c_h eader;

enum usb_offset {
usb_idx_functio n = 0,
usb_idx_size = usb_idx_functio n + sizeof(unsigned char),
usb_idx_i2c_add r = usb_idx_size + sizeof(unsigned char),
usb_idx_i2c_sub addr = usb_idx_i2c_add r + sizeof(unsigned char),
usb_idx_i2c_dat asize = usb_idx_i2c_sub addr + sizeof(unsigned char),
usb_idx_i2c_dat a = usb_idx_i2c_dat asize + sizeof(unsigned char)
};

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

CDC_Read(..., data, MSG_SIZE);

USB_iobuf_commo n_header *hdr = (USB_iobuf_comm on_header *)data;
switch (hdr->function) {
case CMD_I2C_READ:
AT91F_TWI_ReadS ingleIadr(AT91C _BASE_TWI, (usb_idx_i2c_ad dr << 16),
data[4], AT91C_TWI_IADRS Z_1_BYTE, &reg);
...
break;
...
}

At this point I expect to have 'usb_idx_i2c_ad dr' 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: mr*@tusur.ru

[Ravishankar S]

"Roman Mashak" <mr*@tusur.ruwr ote in message
news:fm******** ***@relay.tomsk .ru...

enum usb_offset {
usb_idx_functio n = 0,
usb_idx_size = usb_idx_functio n + sizeof(unsigned char),
usb_idx_i2c_add r = usb_idx_size + sizeof(unsigned char),
usb_idx_i2c_sub addr = usb_idx_i2c_add r + sizeof(unsigned char),
usb_idx_i2c_dat asize = usb_idx_i2c_sub addr + sizeof(unsigned char),
usb_idx_i2c_dat a = usb_idx_i2c_dat asize + sizeof(unsigned char)
};

break;

...
}

At this point I expect to have 'usb_idx_i2c_ad dr' equal to 'data[3]', but

'usb_idx_i2c_ad dr' will be equal to 3 (the index) , not data[3]. enumeration
values _are_ the offsets into
the data[] array.

[Ravishankar S]

"Ravishanka r S" <ra***********@ in.bosch.comwro te in message
news:fm******** **@news4.fe.int ernet.bosch.com ...

>
"Roman Mashak" <mr*@tusur.ruwr ote in message
news:fm******** ***@relay.tomsk .ru...

enum usb_offset {
usb_idx_functio n = 0,
usb_idx_size = usb_idx_functio n + sizeof(unsigned char),
usb_idx_i2c_add r = usb_idx_size + sizeof(unsigned char),
usb_idx_i2c_sub addr = usb_idx_i2c_add r + sizeof(unsigned char),
usb_idx_i2c_dat asize = usb_idx_i2c_sub addr + sizeof(unsigned char),
usb_idx_i2c_dat a = usb_idx_i2c_dat asize + sizeof(unsigned char)
};

break;

...
}

At this point I expect to have 'usb_idx_i2c_ad dr' equal to 'data[3]',

but

values _are_ the offsets into
the data[] array.

Infact 'usb_idx_i2c_ad dr' is not 3 , but 2.
usb_idx_functio n = 0;
usb_idx_size = usb_idx_functio n + sizeof(unsigned char) = 0 + 1 = 1
usb_idx_i2c_add r = usb_idx_size + sizeof(unsigned char) = 1 + 1 = 2

[fnegroni]

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.

[Ravishankar S]

"fnegroni" <f.*********@go oglemail.comwro te in message
news:9d******** *************** ***********@l6g 2000prm.googleg roups.com...

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.

[Ben Bacarisse]

"Ravishanka r S" <ra***********@ in.bosch.comwri tes:

"fnegroni" <f.*********@go oglemail.comwro te in message
news:9d******** *************** ***********@l6g 2000prm.googleg roups.com...

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

[suresh shenoy]

On Jan 9, 9:36*pm, "Roman Mashak" <[e-mail] rote:

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_W RITE

};

struct __attribute__ ((packed)) USB_iobuf_commo n_header_s {
* * unsigned char function;
* * unsigned char size_lo;
* * unsigned char size_hi;

};

typedef struct USB_iobuf_commo n_header_s USB_iobuf_commo n_header;

struct __attribute__ ((packed)) USB_iobuf_i2c_h eader_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_h eader_s USB_iobuf_i2c_h eader;

enum usb_offset {
* * *usb_idx_functi on = 0,
* * *usb_idx_size *= usb_idx_functio n *+ sizeof(unsigned char),
* * *usb_idx_i2c_ad dr = usb_idx_size * + sizeof(unsigned char),
* * *usb_idx_i2c_su baddr = usb_idx_i2c_add r *+ sizeof(unsigned char),
* * *usb_idx_i2c_da tasize = usb_idx_i2c_sub addr + sizeof(unsigned char),
* * *usb_idx_i2c_da ta * * = usb_idx_i2c_dat asize + sizeof(unsigned char)

};

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

CDC_Read(..., data, MSG_SIZE);

USB_iobuf_commo n_header *hdr = (USB_iobuf_comm on_header *)data;
switch (hdr->function) {
* * case CMD_I2C_READ:
* * * * AT91F_TWI_ReadS ingleIadr(AT91C _BASE_TWI, (usb_idx_i2c_ad dr<< 16),
data[4], AT91C_TWI_IADRS Z_1_BYTE, &reg);
* * * * *...
* * * * break;
* * * * ...

}

At this point I expect to have 'usb_idx_i2c_ad dr' 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]

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_h eader header;

read(filehandle , &header, sizeof(USB_iobu f_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

[Ben Bacarisse]

"Roman Mashak" <mr*@tusur.ruwr ites:

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

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

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

??| common_header | data ... |
BBor
??| common_header | some_other_head er | 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_functio n = 0,
usb_idx_size = usb_idx_functio n + sizeof(unsigned char),
usb_idx_i2c_add r = usb_idx_size + sizeof(unsigned short),
usb_idx_i2c_sub addr = usb_idx_i2c_add r + sizeof(unsigned char),
usb_idx_i2c_dat asize = usb_idx_i2c_sub addr + sizeof(unsigned short),
usb_idx_i2c_dat a = usb_idx_i2c_dat asize + 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.

[Roman Mashak]

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

[skip]
BBThen 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
BBheader will consist of 8 chars.

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

BBHaving indexes without access functions seems pointless to me. If you
BBwant to have both the struct and the index enum, why not use offsetof?
BBIt 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: mr*@tusur.ru