netdev - Re: [PATCH net-next v2 01/13] genetlink: refactor the cmd <> policy mapping dump

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: <20221102185230.27ce05b1@kernel.org>
Date:   Wed, 2 Nov 2022 18:52:30 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jacob Keller <jacob.e.keller@...el.com>
Cc:     <davem@...emloft.net>, <netdev@...r.kernel.org>,
        <edumazet@...gle.com>, <pabeni@...hat.com>, <jiri@...nulli.us>,
        <razor@...ckwall.org>, <nicolas.dichtel@...nd.com>,
        <gnault@...hat.com>, <fw@...len.de>
Subject: Re: [PATCH net-next v2 01/13] genetlink: refactor the cmd <> policy
 mapping dump

On Wed, 2 Nov 2022 16:52:21 -0700 Jacob Keller wrote:
> Does the change to ctx->opidx have any other side effects we care about? 
> if not it might be more legible to write this as:
> 
> /* don't modify ctx->opidx */
> }
> 
> while (!ctx->single_op && ctx->opidx < genl_get_cmd_cnt(ctx->r)) {
> 
> 
> That makes the intent a bit more clear and shouldn't need a comment 
> about entering the loop. It also means we don't need to modify 
> ctx->opidx, though I'm not sure if those other side effects matter or 
> not.. we were modifying it before..
> 
> I don't know what else depends on the opidx.

I was just trying to make the patches slightly easier to read.
This chunk gets rewritten again in patch 10, and the opidx thing 
is gone completely. I maintain a "keep dumping" boolean called
dump_map (because this code is dumping a mapping rather than 
the policies which come later)

LMK if I should try harder to improve this patch or what patch 10 
does makes this moot.