From mboxrd@z Thu Jan 1 00:00:00 1970 From: Johannes Berg Subject: Re: [RFC 4/5] netlink: prepare validate extack setting for recursion Date: Wed, 19 Sep 2018 11:44:59 +0200 Message-ID: <1537350299.10305.33.camel@sipsolutions.net> References: <20180918131212.20266-1-johannes@sipsolutions.net> <20180918131212.20266-4-johannes@sipsolutions.net> <20180919111048.092376b2@redhat.com> <1537348525.10305.15.camel@sipsolutions.net> <20180919112829.647896cf@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org To: Jiri Benc Return-path: Received: from s3.sipsolutions.net ([144.76.43.62]:51500 "EHLO sipsolutions.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727056AbeISPWQ (ORCPT ); Wed, 19 Sep 2018 11:22:16 -0400 In-Reply-To: <20180919112829.647896cf@redhat.com> Sender: netdev-owner@vger.kernel.org List-ID: 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