On Tue, 2017-01-10 at 09:21 +0200, Oleksandr Andrushchenko wrote: > On 01/07/2017 12:20 AM, Stefano Stabellini wrote: > > On Fri, 6 Jan 2017, Oleksandr Andrushchenko wrote: > > > |                               reserved                          > > >        | > > > + * +-----------------+-----------------+-----------------+---- > > > -------------+ > > > + * > > > |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\ > > > /\/\/\/| > > > + * +-----------------+-----------------+-----------------+---- > > > -------------+ > > I guess this means that we are skipping some bytes because they are > > all > > reserved, right?  If so, it would be useful to write the byte count > > at this > > point. What's the total size of the event struct? > > > IMO, we shouldn't put any sizes here because: > 1. Above we say "All event packets have the same >     length (40 octets)" > 2. All the event structures are part of the > union xenkbd_in_event, which has > char pad[XENKBD_IN_EVENT_SIZE]; > which effectively regulates the size of the event. > In which case, you can use either 40 or XENKBD_IN_EVENT_SIZE (probably the latter). It's indeed a repetition, but a good one, IMO: it helps the reader, as she won't have to go back to figure out how big the struct was, how the macro was call and to what value it was defined). Regards, Dario -- <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)