Re: [patch 07/11] zfcp: Cleanup of code in zfcp_aux.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> +static int __init zfcp_device_setup(char *devstr)
>  {
> -	char *tmp, *str;
> -	size_t len;
> +	char *token;
> 
>  	if (!devstr)
>  		return 0;
> 
> -	len = strlen(devstr) + 1;
> -	str = kmalloc(len, GFP_KERNEL);
> -	if (!str) {
> -		pr_err("zfcp: Could not allocate memory for "
> -		       "device parameter string, device not attached.\n");
> -		return 0;
> -	}
> -	memcpy(str, devstr, len);
> -
> -	tmp = strchr(str, ',');
> -	if (!tmp)
> +	token = strsep(&devstr, ",");
> +	if (!token || strlen(token) >= BUS_ID_SIZE)
>  		goto err_out;
> -	*tmp++ = '\0';
> -	strncpy(zfcp_data.init_busid, str, BUS_ID_SIZE);
> -	zfcp_data.init_busid[BUS_ID_SIZE-1] = '\0';
> +	strncpy(zfcp_data.init_busid, token, BUS_ID_SIZE);
> 
> -	zfcp_data.init_wwpn = simple_strtoull(tmp, &tmp, 0);
> -	if (*tmp++ != ',')
> -		goto err_out;
> -	if (*tmp == '\0')
> +	token = strsep(&devstr, ",");
> +	if (!token || strict_strtoull(token, 0, &zfcp_data.init_wwpn))
>  		goto err_out;
> 
> -	zfcp_data.init_fcp_lun = simple_strtoull(tmp, &tmp, 0);
> -	if (*tmp != '\0')
> +	token = strsep(&devstr, ",");
> +	if (!token || strict_strtoull(token, 0, &zfcp_data.init_fcp_lun))
>  		goto err_out;
> -	kfree(str);
>  	return 1;
> 
>   err_out:
>  	pr_err("zfcp: Parse error for device parameter string %s, "
> -	       "device not attached.\n", str);
> -	kfree(str);
> +	       "device not attached.\n", devstr);
>  	return 0;
>  }

Sorry, but this still seems to be incorrect. strsep modifies the string that
gets passed to it. In this case you let it operate on the original module
parameter string which is also exported via sysfs.
So after parsing finished the exported string is much shorter than the
original string. That was actually the whole point why the old code allocated
memory and copied the string so the copy could be modified.
A comment would have been good ;)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux