Re: [PATCH v4] cmdfilter: clean up sysfs interface

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

 



On Fri, Aug 08, 2008 at 11:56:35PM +0200, Adel Gadllah wrote:
> @@ -84,8 +83,8 @@ static ssize_t rcf_cmds_show(struct
> blk_scsi_cmd_filter *filter, char *page,
> 
>  	for (i = 0; i < BLK_SCSI_MAX_CMDS; i++) {
>  		if (test_bit(i, okbits)) {
> -			sprintf(npage, "%02x", i);
> -			npage += 2;
> +			sprintf(npage, "0x%02x", i);
> +			npage += 4;
>  			if (i < BLK_SCSI_MAX_CMDS - 1)
>  				sprintf(npage++, " ");
>  		}

Why not:

			npage += sprintf(npage, "0x%02x", i);
?  (and before anyone suggests snprintf, we can at most have 256
entries, each consuming 5 bytes.  5 * 256 <3k and no arch has less than
a 4k page size.  so we can't overflow the buffer.)

> @@ -111,32 +110,41 @@ static ssize_t rcf_writecmds_show(struct
> blk_scsi_cmd_filter *filter,
>  static ssize_t rcf_cmds_store(struct blk_scsi_cmd_filter *filter,
>  			      const char *page, size_t count, int rw)
>  {
> -	ssize_t ret = 0;
>  	unsigned long okbits[BLK_SCSI_CMD_PER_LONG], *target_okbits;
> -	int cmd, status, len;
> -	substring_t ss;
> -
> -	memset(&okbits, 0, sizeof(okbits));
> -
> -	for (len = strlen(page); len > 0; len -= 3) {
> -		if (len < 2)
> -			break;
> -		ss.from = (char *) page + ret;
> -		ss.to = (char *) page + ret + 2;
> -		ret += 3;
> -		status = match_hex(&ss, &cmd);
> +	int cmd, set;
> +	char *p, *status;
> +
> +	if (rw == READ) {
> +		memcpy(&okbits, filter->read_ok, sizeof(okbits));
> +		target_okbits = filter->read_ok;
> +	} else {
> +		memcpy(&okbits, filter->write_ok, sizeof(okbits));
> +		target_okbits = filter->write_ok;
> +	}
> +
> +	while ((p = strsep((char **)&page, " ")) != NULL) {
> +		set = 1;
> +
> +		if (p[0] == '-') {
> +			set = 0;
> +			p++;
> +		}
> +
> +		if (p[0] == '+')
> +			p++;
> +
> +		cmd = simple_strtol(p, &status, 16);
> +
>  		/* either of these cases means invalid input, so do nothing. */
> -		if (status || cmd >= BLK_SCSI_MAX_CMDS)
> +		if ((status == p) || cmd >= BLK_SCSI_MAX_CMDS)
>  			return -EINVAL;

Doesn't this accept -+0x20 for example?  Why not:

		set = 1;
		if (p[0] == '+') {
			p++;
		} else if (p[0] == '-') {
			set = 0;
			p++;
		}
		cmd = simple_strtol(p, &status, 16);

> -		__set_bit(cmd, okbits);
> +		if (set)
> +			__set_bit(cmd, okbits);
> +		else
> +			__clear_bit(cmd, okbits);
>  	}
> 
> -	if (rw == READ)
> -		target_okbits = filter->read_ok;
> -	else
> -		target_okbits = filter->write_ok;
> -
>  	memmove(target_okbits, okbits, sizeof(okbits));

target_okbits can't overlap with okbits, so you can use memcpy instead
of memmove here.

>  	return count;
>  }

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
--
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