Re: [Qemu-devel] [[RFC]PATCH:hw/sd:sd_init()] hw/sd : modified the sd_init() function

[Tejaswini Poluri <tejaswinipoluri3@gmail.com>]

On Mon, Mar 27, 2017 at 4:43 PM, Stefan Hajnoczi <stefanha@redhat.com>
wrote:

> On Mon, Mar 27, 2017 at 04:01:02PM +0530, Tejaswini wrote:
> > From: Tejaswini Poluri <tejaswinipoluri3@gmail.com>
>
> Please shorten the subject line: "[PATCH] hw/sd: simplify sd_init()
> prototype"
>
> > @@ -573,16 +573,19 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
> >      qdev_prop_set_drive(dev, "drive", blk, &err);
> >      if (err) {
> >          error_report("sd_init failed: %s", error_get_pretty(err));
> > -        return NULL;
> > +        return -1;
> >      }
> >      qdev_prop_set_bit(dev, "spi", is_spi);
> >      object_property_set_bool(obj, true, "realized", &err);
> >      if (err) {
> >          error_report("sd_init failed: %s", error_get_pretty(err));
> > -        return NULL;
> > +        return -1;
> >      }
> > -
> > -    return SD_CARD(dev);
> > +    sd_state = SD_CARD(dev);
>
> The caller will not see the new value of sd_state.  In C arguments are
> passed by value.  That means they are local variables inside the
> function and do not affect the caller.
>
> The caller will call sd_init() along with passing SDState pointer as an
argument like below
if (sd_init(s->card, blk, false) < 0) .
And the sd_init() function is modified to
int sd_init(SDState *sd_state, BlockBackend *blk, bool is_spi)
so that the caller gets the new value of SDstate updated.
Looking forward for the comments of Paolo Bonzini to understand what more
needs to be done as part of the task.

I have CCed Paolo Bonzini, who posted this task.  Maybe he can explain
> what he meant by "Include SDState by value instead of allocating it in
> sd_init (hw/sd/)".
>
> > +    if (!sd_state) {
> > +             return -1;
> > +     }
>
> QEMU use 4 space indentation.  Please do not use tabs.
>



-- 
Regards,
Tejaswini