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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170731122106.GD1883@nanopsycho>
Date:   Mon, 31 Jul 2017 14:21:06 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jamal Hadi Salim <jhs@...atatu.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org,
        xiyou.wangcong@...il.com, eric.dumazet@...il.com,
        horms@...ge.net.au, dsahern@...il.com
Subject: Re: [PATCH net-next v12 1/4] net netlink: Add new type NLA_BITFIELD32

Mon, Jul 31, 2017 at 02:03:55PM CEST, jhs@...atatu.com wrote:
>On 17-07-31 02:38 AM, Jiri Pirko wrote:
>> Sun, Jul 30, 2017 at 09:59:10PM CEST, jhs@...atatu.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.