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