2008/8/8 FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx>: > On Fri, 8 Aug 2008 20:46:55 +0200 > "Adel Gadllah" <adel.gadllah@xxxxxxxxx> wrote: > >> v3 clean up parsing. >> >> ----------------------------------- >> >> This patch changes the interface of the cmd filter to use a +/- notation like: >> echo -- +0x02 +0x03 -0x08 >> If neither + or - is given it defaults to + (allow command). >> >> Note: The interface was added in 2.6.17-rc1 and is unused >> and undocumented so far so it's safe to change it. >> >> Cc: matthew@xxxxxx >> Cc: fujita.tomonori@xxxxxxxxxxxxx, >> Cc: jens.axboe@xxxxxxxxxx >> Cc: James.Bottomley@xxxxxxxxxxxxxxxxxxxxx >> Cc: dan.j.williams@xxxxxxxxx >> Cc: pjones@xxxxxxxxxx >> Cc: viro@xxxxxxxxxxxxxxxxxx >> Cc: dougg@xxxxxxxxxx >> Signed-off-by: Adel Gadllah <adel.gadllah@xxxxxxxxx> >> >> block/cmd-filter.c | 53 ++++++++++++++++++++++++++++++--------------------- >> 1 files changed, 31 insertions(+), 22 deletions(-) > > Hmm, the patch seems to be corrupted again. OK, attached as file... > >> diff --git a/block/cmd-filter.c b/block/cmd-filter.c >> index eec4404..f838d04 100644 >> --- a/block/cmd-filter.c >> +++ b/block/cmd-filter.c > > I think that you can remove "linux/parser.h". > > >> @@ -84,8 +84,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++, " "); >> } >> @@ -111,32 +111,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; >> + } > > Do we need this? We can modify filter->write_ok or filter->read_ok > directly? Yes, because in case of an error (wrong input) only a part of the request will be done (ex: enable 2 commands but not the third due to a typo which might confuse the user). >> + while ((p = strsep((char **)&page, " ")) != NULL) { >> + set = 1; >> + >> + if (p[0] == '-') { >> + set = 0; >> + p++; >> + } > > As I wrote, I prefer to let simple_strtol to handle '-' though the > current code is also fine by me. well this seems easier (see my last mail)
Attachment:
cmd-filter-new-iface.patch
Description: Binary data