Hii Paolo, Waiting for your comments on this patch Regards, Tejaswini On 28 Mar 2017 11:01 a.m., "Tejaswini Poluri" <tejaswinipolu...@gmail.com> wrote:
> > > On Mon, Mar 27, 2017 at 4:43 PM, Stefan Hajnoczi <stefa...@redhat.com> > wrote: > >> On Mon, Mar 27, 2017 at 04:01:02PM +0530, Tejaswini wrote: >> > From: Tejaswini Poluri <tejaswinipolu...@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 >