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

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

 



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


[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