Re: [PATCH v6 net-next 1/7] netlink: Add a macro to set policy message with format string

[Jakub Kicinski <kuba@kernel.org>]

On Sat, 25 Mar 2023 16:49:34 +0300 Shay Agroskin wrote:
> > TBH the hierarchy should be the other way around, 
> > NL_SET_ERR_MSG_FMT()
> > should be converted to be:
> >
> > #define NL_SET_ERR_MSG_FMT(extack, attr, msg, args...) \
> >         NL_SET_ERR_MSG_ATTR_POL_FMT(extack, NULL, NULL, msg, 
> >         ##args)
> >
> > and that'd fix the shadowing, right?  
> 
> Well ... It will but it will contradict the current order of calls 
> as I see it.
> 
> NL_SET_ERR_MSG_FMT_MOD calls NL_SET_ERR_MSG_FMT which can be 
> described as 'the former extends the latter'.
> 
> On the other hand NL_SET_ERR_MSG_ATTR_POL implements the message 
> setting by itself and doesn't use NL_SET_ERR_MSG to set the 
> message.
> 
> So it seems like we already have two approaches for macro ordering 
> here and following your suggestion would create another type of 
> call direction of 'the former shrinks the latter by setting to 
> NULL its attributes'.
> Overall given the nature of C macros and the weird issues arise by 
> shadowing variables (ending up for some reason in not modifying 
> the pointer at all..) I'd say I mostly prefer V7 version which 
> re-implements the message setting and avoids creating such very 
> hard to find issues later.
> 
> Then again I'd follow your implementation suggestion if you still 
> prefer it (also I can modify the macro NL_SET_ERR_MSG to call 
> NL_SET_ERR_MSG_ATTR_POL to be consistent with the other change)

Alright, it doesn't matter all that much.