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

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

 



On Tuesday 01 July 2008 00:52, Heiko Carstens wrote:
> > +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 ;)
> --
Correct,

I fixed the patch and Christof will send it soon to the list.

Cheers Swen

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