On Mon, Jul 28, 2014 at 09:22:36AM -0400, Benjamin Romer wrote:
> @@ -1736,53 +1748,36 @@ parahotplug_process_message(CONTROLVM_MESSAGE *inmsg)
> }
> }
>
> -/*
> - * Gets called when the udev script writes to
> - * /proc/visorchipset/parahotplug. Expects input in the form of "<id>
> - * <active>" where <id> is the identifier passed to the script that
> - * matches a request on the request list, and <active> is 0 or 1
> - * indicating whether the device is now enabled or not.
> +/* The parahotplug/devicedisabled interface gets called by our support script
> + * when an SR-IOV device has been shut down. The ID is passed to the script
> + * and then passed back when the device has been removed.
> */
> -static ssize_t
> -parahotplug_proc_write(struct file *file, const char __user *buffer,
> - size_t count, loff_t *ppos)
> +static ssize_t devicedisabled_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> {
> - char buf[64];
> uint id;
> - ushort active;
>
> - if (count > sizeof(buf) - 1) {
> - LOGERR("parahotplug_proc_write: count (%d) exceeds size of buffer (%d)",
> - (int) count, (int) sizeof(buf));
> + if (kstrtouint(buf, 10, &id) != 1)
> return -EINVAL;
> - }
This is the same bug I mentioned ealier in a different patch.
> - if (copy_from_user(buf, buffer, count)) {
> - LOGERR("parahotplug_proc_write: copy_from_user failed");
> - return -EFAULT;
> - }
> - buf[count] = '\0';
> + parahotplug_request_complete((int) id, 0);
This cast is not needed. The kernel also has a kstrtoint() function, if
you would prefer to use that.
> + return count;
> +}
>
> - if (sscanf(buf, "%u %hu", &id, &active) != 2) {
> - id = 0;
> - active = 0;
> - }
> +/* The parahotplug/deviceenabled interface gets called by our support script
> + * when an SR-IOV device has been recovered. The ID is passed to the script
> + * and then passed back when the device has been brought back up.
> + */
> +static ssize_t deviceenabled_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + uint id;
>
> - if (active != 1 && active != 0) {
> - LOGERR("parahotplug_proc_write: invalid active field");
> + if (kstrtouint(buf, 10, &id) != 1)
> return -EINVAL;
> - }
Nope.
> -
> - parahotplug_request_complete((int) id, (U16) active);
> -
> + parahotplug_request_complete((int) id, 1);
Remove the case. Also there should be no space between the cast and the
thing being casted to remind you that casting is a high precedence
operation.
> return count;
> }
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
