Subject: Re: [RFC 4/5] netlink: prepare validate extack setting for recursion

From: Johannes Berg <[email-masked]>
Date: Wed, 19 Sep 2018 11:44:59 +0200

On Wed, 2018-09-19 at 11:28 +0200, Jiri Benc wrote:

> > It might be possible to do this differently, in theory, but all the ways
> > I've tried to come up with so far made the code vastly more complex.
> 
> Wouldn't still make sense to store the flag in the struct
> netlink_ext_ack, though?

Does it make sense to store a flag there that only has a single,
localized, user? I'd say no.

> The way the parameters are passed around in
> this patch looks ugly. 

Yeah, it's not the best in some ways.

I considered having a cleared new extack on the stack in the outer-most
call (nla_parse/nla_validate), and then we can "set if not already set",
and at the end unconditionally copy/set to the real one... But then I
either need to duplicate that code in both, or pass another argument to
nla_validate_parse() anyway to indicate whether to do this or not (since
the inner calls to it shouldn't, since that would defeat the purpose),
or add another level of function call indirection?

I'm not really sure any of these are better ... and more complex, in
some ways, since we have to copy all the data around.

> And adding more users of the flag later (there
> may be other cases when we want the earlier messages to be preserved)
> would mean adding parameters all around, while the flag in the struct
> would be readily available.

I can't really think of any other users of such a thing?

johannes