On Fri, 31 Jan 2014, Dan Williams wrote:

> For example:

Topic sentence?  A reader seeing this could be forgiven for wondering
"Example of what?".

> usb2/2-1/2-1:1.0/port1/peer => ../../../../usb3/3-1/3-1:1.0/port1
> usb2/2-1/2-1:1.0/port2/peer => ../../../../usb3/3-1/3-1:1.0/port2
> usb2/2-1/2-1:1.0/port3/peer => ../../../../usb3/3-1/3-1:1.0/port3
> usb2/2-1/2-1:1.0/port4/peer => ../../../../usb3/3-1/3-1:1.0/port4
> usb2/2-0:1.0/port1/peer     => ../../../usb3/3-0:1.0/port1
> usb2/2-0:1.0/port2/peer     => ../../../usb3/3-0:1.0/port2
> usb2/2-0:1.0/port3/peer     => ../../../usb3/3-0:1.0/port3
> usb2/2-0:1.0/port4/peer     => ../../../usb3/3-0:1.0/port4
> 
> usb3/3-1/3-1:1.0/port1/peer => ../../../../usb2/2-1/2-1:1.0/port1
> usb3/3-1/3-1:1.0/port2/peer => ../../../../usb2/2-1/2-1:1.0/port2
> usb3/3-1/3-1:1.0/port3/peer => ../../../../usb2/2-1/2-1:1.0/port3
> usb3/3-1/3-1:1.0/port4/peer => ../../../../usb2/2-1/2-1:1.0/port4
> usb3/3-0:1.0/port1/peer     => ../../../usb2/2-0:1.0/port1
> usb3/3-0:1.0/port2/peer     => ../../../usb2/2-0:1.0/port2
> usb3/3-0:1.0/port3/peer     => ../../../usb2/2-0:1.0/port3
> usb3/3-0:1.0/port4/peer     => ../../../usb2/2-0:1.0/port4

> +     mutex_lock(&peer_lock);
> +     peer = port_dev->peer;
> +     do if (peer) {
> +             retval = sysfs_create_link(&port_dev->dev.kobj,
> +                                        &peer->dev.kobj, "peer");
> +             if (retval)
> +                     break;
> +             retval = sysfs_create_link(&peer->dev.kobj,
> +                                        &port_dev->dev.kobj, "peer");
> +     } while (0);

This is an unusual idiom; I don't recall seeing it before.  How about 
writing this in a more conventional fashion?  It would require one 
less line of code...

> +     mutex_unlock(&peer_lock);
> +     if (retval)
> +             goto error_links;
> +
>       pm_runtime_set_active(&port_dev->dev);
>  
>       /* It would be dangerous if user space couldn't
> @@ -339,6 +357,8 @@ int usb_hub_create_port_device(struct usb_hub *hub, int 
> port1)
>  
>  error_register:
>       put_device(&port_dev->dev);
> +error_links:
> +     device_unregister(&port_dev->dev);

You don't explicitly remove the first link if the second link fails.  I 
guess sysfs is smart enough to do this for you, right?

Alan Stern

--
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

Reply via email to