Re: [PATCH] staging: comedi: ni_*

From: Aniruddha Shastri
Date: Thu Dec 14 2017 - 02:48:48 EST


On Wed, Dec 13, 2017 at 6:07 AM, Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, Dec 13, 2017 at 05:16:13PM +0530, Aniruddha Shastri wrote:
>> On Wed, Dec 13, 2017 at 4:20 PM, Greg Kroah-Hartman
>> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>> >
>> > On Wed, Dec 13, 2017 at 04:01:04AM -0600, Aniruddha Shastri wrote:
>> > > Fix checkpatch warnings by shortening lines and reorganizing code where needed..
>> > > Re-phrase the assert messages in ni_mio_common.c. This was done to meet the character limit for the message.
>> >
>> > And yet this line is over the character length :)
>> Aniruddha: Thanks for pointing this out, I'll amend the commit message. :)
>
> Why put your name in the response? Of course I know it's you who is
> writing this :)
I thought it might be clearer for others to read our exchange, but that only
works if everyone does the same. Otherwise, it's just clutter. I'm used to
seeing a name and timestamp next to comments in other code review
systems (like TFS), but this doesn't appear to be the Linux way :)
>
>> > > range_table_list = kmalloc_array(32,
>> > > - sizeof(struct comedi_lrange *),
>> > > + range_size,
>> >
>> > Not worth changing.
>>
>> Aniruddha: The original checkpatch.pl warning instructed to use
>> const struct comedi_lrange instead of struct. Adding the 'const' put this
>> line over the limit, so that's why I pulled it out into a local variable.
>
> Checkpatch is a "hint", don't always follow it, it can cause some code
> to look really bad. It's not a hard-rule.
That's a relief :). I've sent updated patches that only clean-up where
needed.
>
> thanks,
>
> greg k-h