> -----Original Message----- > From: Greg KH [mailto:greg@xxxxxxxxx] > Sent: Tuesday, April 26, 2011 6:51 PM > To: KY Srinivasan > Cc: gregkh@xxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > devel@xxxxxxxxxxxxxxxxxxxxxx; virtualization@xxxxxxxxxxxxxx; Haiyang Zhang; > Abhishek Kane (Mindtree Consulting PVT LTD) > Subject: Re: [PATCH 12/25] Staging: hv: Cleanup error handling in > vmbus_child_device_register() > > On Tue, Apr 26, 2011 at 09:20:29AM -0700, K. Y. Srinivasan wrote: > > Cleanup error handling in vmbus_child_device_register(). > > > > Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx> > > Signed-off-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx> > > Signed-off-by: Abhishek Kane <v-abkane@xxxxxxxxxxxxx> > > Signed-off-by: Hank Janssen <hjanssen@xxxxxxxxxxxxx> > > --- > > drivers/staging/hv/vmbus_drv.c | 7 ++++++- > > 1 files changed, 6 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c > > index d597dd4..4d569ad 100644 > > --- a/drivers/staging/hv/vmbus_drv.c > > +++ b/drivers/staging/hv/vmbus_drv.c > > @@ -720,11 +720,16 @@ int vmbus_child_device_register(struct hv_device > *child_device_obj) > > */ > > ret = device_register(&child_device_obj->device); > > > > + if (ret) > > + return ret; > > + > > /* vmbus_probe() error does not get propergate to device_register(). */ > > ret = child_device_obj->probe_error; > > Wait, why not? Why is the probe_error have to be saved off like this? > That seems like something is wrong here, this patch should not be > needed. > > Well, you should check the return value of device_register, that is > needed, but this seems broken somehow. The current code had comments claiming that the probe error was not correctly propagated. Looking at the kernel side of the code, it was not clear if device_register() could succeed while the probe might fail. In any event, if you can guarantee that device_register() can return any probe related errors, I agree with you that saving the probe error is an overkill. The current code saved the probe error and with new check I added with regards to the return value of device_register, there is no correctness issue with this patch. > > > > > - if (ret) > > + if (ret) { > > pr_err("Unable to register child device\n"); > > + device_unregister(&child_device_obj->device); > > + } > > else > > } else > is the preferred way. > > Care to send a fixup patch to remove the probe_error field and fix this > formatting error up? I will fix up the formatting issue. Regards, K. Y _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
