> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> Sent: Tuesday, April 26, 2011 6:51 PM
> To: KY Srinivasan
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; 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 <[email protected]>
> > Signed-off-by: Haiyang Zhang <[email protected]>
> > Signed-off-by: Abhishek Kane <[email protected]>
> > Signed-off-by: Hank Janssen <[email protected]>
> > ---
> > 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
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
