[Qemu-devel] Re: [PATCH 4/5] atapi: GESN: Add enums for commonly-used field types

[Amit Shah <amit.shah@redhat.com>]

On (Fri) 08 Apr 2011 [16:21:38], Kevin Wolf wrote:
> Am 08.04.2011 09:15, schrieb Amit Shah:
> > Instead of using magic numbers, use enums that are more descriptive of
> > the fields being used.
> > 
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > ---
> >  hw/ide/core.c |   11 +++++++++--
> >  1 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/ide/core.c b/hw/ide/core.c
> > index 730587e..cdc2c56 100644
> > --- a/hw/ide/core.c
> > +++ b/hw/ide/core.c
> > @@ -1118,12 +1118,19 @@ static void handle_get_event_status_notification(IDEState *s,
> >                                                   uint8_t *buf,
> >                                                   const uint8_t *packet)
> >  {
> > +    enum cdb {
> > +        polled = 1,
> > +        request = 4,
> > +        allocation_length_msb = 7,
> > +        allocation_length_lsb = 8,
> > +        control = 9,
> > +    };
> 
> Wouldn't it be nicer to make this a struct and just cast packet to a
> pointer to this struct? At first I didn't realize that this should be
> field offsets and I find something like packet + allocation_length_msb
> rather confusing.

Yes, indeed.  It was faster for me to prototype it this way, but I
will convert it to a struct.

		Amit