Re: [RFC PATCH v2 3/5] libfcoe, fcoe, bnx2fc: Add new fcoe control interface

[Bart Van Assche <bvanassche@acm.org>]

On 09/27/12 04:01, Robert Love wrote:
> +static ssize_t store_ctlr_enabled(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t count)
> +{
> +	struct fcoe_ctlr_device *ctlr = dev_to_ctlr(dev);
> +	int val;
> +	int rc;
> +
> +	rc = sscanf(buf, "%d", &val);
> +	if (!rc)
> +		return -EINVAL;

sscanf() expects a '\0'-terminated buffer which is not guaranteed by the 
caller of this function (sysfs), isn't it ?

> @@ -830,6 +983,18 @@ int __init fcoe_sysfs_setup(void)
>   	if (error)
>   		return error;
>
> +	error = bus_create_file(&fcoe_bus_type, &bus_attr_ctlr_create);
> +	if (error) {
> +		bus_unregister(&fcoe_bus_type);
> +		return error;
> +	}
> +
> +	error = bus_create_file(&fcoe_bus_type, &bus_attr_ctlr_destroy);
> +	if (error) {
> +		bus_unregister(&fcoe_bus_type);
> +		return error;
> +	}
> +
>   	return 0;
>   }

It might be a good idea to use fcoe_bus_type.bus_attrs instead of 
bus_create_file(). If someone ever would want to trigger these 
attributes from udev immediately after an fcoe bus instance has been 
created then that approach will avoid a race where udev gets notified 
before these attributes got created.

>   static int fcoe_add_netdev_mapping(struct net_device *netdev,
> -					struct fcoe_transport *ft)
> +				   struct fcoe_transport *ft)

Is the above whitespace change necessary ?

Bart.