Re: [Qemu-devel] [PATCH 2/3] hw/sd.c: add SD card save/load support

[Mitsyanko Igor]

On 12/27/2011 05:27 PM, Andreas Färber wrote:
> Am 26.12.2011 15:58, schrieb Peter Maydell:
> > > diff --git a/hw/sd.c b/hw/sd.c
> > > index 07eb263..2b489d3 100644
> > > --- a/hw/sd.c
> > > +++ b/hw/sd.c
>
> > > @@ -81,22 +85,22 @@ struct SDState {
> > >      uint8_t sd_status[64];
> > >      uint32_t vhs;
> > >      int wp_switch;
> > > -    int *wp_groups;
> > > +    uint8_t *wp_groups;
> > >      uint64_t size;
> > > -    int blk_len;
> > > +    uint32_t blk_len;
> > >      uint32_t erase_start;
> > >      uint32_t erase_end;
> > >      uint8_t pwd[16];
> > > -    int pwd_len;
> > > -    int function_group[6];
> > > +    uint32_t pwd_len;
> > > +    uint8_t function_group[6];
> > >
> > > -    int spi;
> > > -    int current_cmd;
> > > +    uint8_t spi;
> > > +    uint8_t current_cmd;
> > >      /* True if we will handle the next command as an ACMD. Note that this does
> > >       * *not* track the APP_CMD status bit!
> > >       */
> > > -    int expecting_acmd;
> > > -    int blk_written;
> > > +    bool expecting_acmd;
> >
> > Why have you changed expecting_acmd and enable to bool, but spi
> > to a uint8_t and wp_groups[] to a uint8_t[] ? This isn't very
> > consistent. (I'm vaguely against bool because you then end up
> > making other changes to change '1' to 'true' and so on.)
>
> > > @@ -1706,5 +1710,44 @@ int sd_data_ready(SDState *sd)
> > >
> > >   void sd_enable(SDState *sd, int enable)
> > >   {
> > > -    sd->enable = enable;
> > > +    sd->enable = enable ? true : false;
> >
> > This kind of thing is why I don't like bool :-)
>
> x = 1 should work with bool when not doing x == true anywhere, here
> !!enable would be an alternative. Maybe change int enable to bool, too?
>
> Andreas

As Peter mentioned previously, we should use either uint8_t or bool for 
all binary variables for consistency, and we probably shouldn't use bool 
for wp_groups array since sizeof(bool) uncertainty. So I'm considering 
just make everything uint8_t.

--
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: address@hidden