Re: [PATCH net-next v12 1/4] net netlink: Add new type NLA_BITFIELD32

[Jiri Pirko <jiri@resnulli.us>]

Mon, Jul 31, 2017 at 02:03:55PM CEST, jhs@mojatatu.com wrote:
>On 17-07-31 02:38 AM, Jiri Pirko wrote:
>> Sun, Jul 30, 2017 at 09:59:10PM CEST, jhs@mojatatu.com wrote:
>> > Jiri,
>> > 
>> > This is getting exhausting, seriously.
>> > I posted the code you are commenting one two days ago so i dont have to
>> > repost.
>> 
>> And I commented on the "*u32 = *u32" thing. But you ignored it. Pardon
>> me for mentioning that again now :/
>> 
>
>You commented on *u32 assignment from *void which i fixed. I
>intentionally selected the different assignment names to reflect
>meaning. Had you commented earlier - although I would have found

Yep, I don't understand why the function arg cannot have the desired
name right away. Also, I don't understand why you don't just have u32
instead of pointer as a local variable, if you really needed this local
variable. Ok, I admit that ":)" is probably not intuitive comment.
Will be more blunt next time.


>it disagreable - I would have fixed that too. Jiri, you need to be
>more tolerant so progress can be made at times.

I don't think so. I believe that it is really important that code can be
read nicely. If we don't do it, it will be just mess (like it is in lot
of net/sched/ places).


>
>> 
>> > 
>> > On D. Ahern: I dont think we are disagreeing anymore on the need to
>> > generalize the check. He is saying it should be a helper and I already
>> > had the validation data; either works. I dont see the gapping need
>> > to remove the validation data.
>> 
>> DavidA? Your opinion.
>> 
>
>With DavidA(reading his response) - the issue is one of taste.
>Again either approach is fine. You can call helpers for every user
>or make them invoked behind the scenes.
>Again - like all your comments on code taste which I addressed, I
>would have made that change if the comment had come in earlier. I got
>exhausted. Imagine how a newbie corporate guy wouldve felt after this.

That's how it is.