Re: [PATCH 3/4] usb: Create link files between child device and usb port device.

[Lan Tianyu]

On 2012/10/25 5:49, Greg KH wrote:

On Wed, Oct 24, 2012 at 09:24:04PM +0800, Lan Tianyu wrote:

于 2012/10/24 18:55, Sergei Shtylyov 写道:

+    /* Create link files between child device and usb port device. */
+    if (udev->parent) {
+        int no_warn;

    I think 'ret' or 'result' is a better name...

+        struct usb_port *port_dev =
+            hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
+
+        no_warn = sysfs_create_link(&udev->dev.kobj,
+                &port_dev->dev.kobj, "port");
+        no_warn = sysfs_create_link(&port_dev->dev.kobj,
+                &udev->dev.kobj, "child");

     I guess you are not supposed to ignore the result if the function requires
it not to be ignored.

Hi Sergei:
        Great thanks for your review. From my opinion, failure to create link 
will
not affect usb device function and so the return value can be ignored, Perharps
producing
some warning will be better. Do you have some suggestion? :)

Properly handle the error, don't ignore it.

HI Greg:
        Ok, I get it. How about following?
+       /* Create link files between child device and usb port device. */
+       if (udev->parent) {
+               struct usb_port *port_dev =
+                       hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
+
+               err = sysfs_create_link(&udev->dev.kobj,
+                               &port_dev->dev.kobj, "port");
+               if (err)
+                       goto fail;
+
+               err = sysfs_create_link(&port_dev->dev.kobj,
+                               &udev->dev.kobj, "child");
+               if (err) {
+                       sysfs_remove_link(&udev->dev.kobj, "port");
+                       goto fail;
+               }
+       }
+

greg k-h

--
Best Regards
Tianyu Lan
linux kernel enabling team
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html