Re: [PATCH v2 2/2] libata-scsi: do not respond with "invalid field" for FORMAT UNIT

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

 



Um it's not mainly about in caps or not, but more about wrongly spelled as cbd.

On 5 July 2016 at 22:39, Tejun Heo <tj@xxxxxxxxxx> wrote:
> Hello,
>
> On Wed, Jul 06, 2016 at 03:13:31AM +0800, Tom Yan wrote:
>> I just thought that such a minor change in a comment can fit in the
>> same patch where the issue was first noticed. Anyway, will split them
>> if I am going to send a v3 set.
>>
>> On 5 July 2016 at 19:08, Sergei Shtylyov
>> <sergei.shtylyov@xxxxxxxxxxxxxxxxxx> wrote:
>> > On 7/5/2016 9:45 AM, tom.ty89@xxxxxxxxx wrote:
>> >
>> >> From: Tom Yan <tom.ty89@xxxxxxxxx>
>> >>
>> >> It does not make sense and is confusing to respond with "Invalid
>> >> field in CDB" while we have no support at all implemented for
>> >> FORMAT UNIT. It is decent to let it go to the default, which
>> >> will respond with "Invalid command operation code" instead.
>> >>
>> >> Signed-off-by: Tom Yan <tom.ty89@xxxxxxxxx>
>> >>
>> >> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>> >> index 029e738..ac5676e 100644
>> >> --- a/drivers/ata/libata-scsi.c
>> >> +++ b/drivers/ata/libata-scsi.c
>> >> @@ -307,7 +307,7 @@ static void ata_scsi_set_invalid_field(struct
>> >> ata_device *dev,
>> >>                                        struct scsi_cmnd *cmd, u16 field,
>> >> u8 bit)
>> >>  {
>> >>         ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x24, 0x0);
>> >> -       /* "Invalid field in cbd" */
>> >> +       /* "Invalid field in CDB" */
>> >
>> >
>> >    Don't do 2 things in one patch please> This change wasn't even documented
>> > in the patch description.
>
> So, for trivial cleanups, it's fine to do it along with other changes
> when the cleanups and the said changes are related and close by;
> however, the description should be able to encompass all changes.  It
> doesn't have to be super detailed.  Something like "While at it, do
> some trivial / typo / whatever cleanups" works fine.
>
> Here, the cleanup isn't that close.  I'd just drop that chunk.  It
> really doesn't matter whether cdb is in caps or not.
>
> Thanks.
>
> --
> tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux