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
| ||
|
Date: Wed, 19 Oct 2022 09:50:16 +0200 From: Johannes Berg <johannes@...solutions.net> To: Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net Cc: netdev@...r.kernel.org, edumazet@...gle.com, pabeni@...hat.com, jiri@...nulli.us, razor@...ckwall.org, nicolas.dichtel@...nd.com, gnault@...hat.com, jacob.e.keller@...el.com, fw@...len.de Subject: Re: [PATCH net-next 01/13] genetlink: refactor the cmd <> policy mapping dump On Tue, 2022-10-18 at 16:07 -0700, Jakub Kicinski wrote: > The code at the top of ctrl_dumppolicy() dumps mappings between > ops and policies. It supports dumping both the entire family and > single op if dump is filtered. But both of those cases are handled > inside a loop, which makes the logic harder to follow and change. > Refactor to split the two cases more clearly. Hmm. Yeah, fair, it's nicer now :) However, > if (!ctx->policies) { > - while (ctx->opidx < genl_get_cmd_cnt(ctx->rt)) { > - struct genl_ops op; > + struct genl_ops op; > > - if (ctx->single_op) { > - int err; > + if (ctx->single_op) { > + int err; > > - err = genl_get_cmd(ctx->op, ctx->rt, &op); > - if (WARN_ON(err)) > - return skb->len; > + err = genl_get_cmd(ctx->op, ctx->rt, &op); > + if (WARN_ON(err)) > + return err; > > - /* break out of the loop after this one */ > - ctx->opidx = genl_get_cmd_cnt(ctx->rt); > - } else { > - genl_get_cmd_by_index(ctx->opidx, ctx->rt, &op); > - } > + if (ctrl_dumppolicy_put_op(skb, cb, &op)) > + return skb->len; > + > + ctx->opidx = genl_get_cmd_cnt(ctx->rt); This (now without a comment that you removed rather than changed), still strikes me as odd. I guess if we add a comment /* don't enter the loop below */ that'd be nicer, but I feel maybe putting the loop into the else instead would be nicer? johannes
Powered by blists - more mailing lists