Re: [nft PATCH 10/28] datatype: Increase symbolic constant printer robustness

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Oct 30, 2025 at 11:35:40PM +0100, Phil Sutter wrote:
> On Thu, Oct 30, 2025 at 10:56:04PM +0100, Pablo Neira Ayuso wrote:
> > On Thu, Oct 30, 2025 at 12:00:48PM +0100, Phil Sutter wrote:
> > > On Wed, Oct 29, 2025 at 07:36:34PM +0100, Pablo Neira Ayuso wrote:
> > > > On Thu, Oct 23, 2025 at 06:13:59PM +0200, Phil Sutter wrote:
> > > > > Do not segfault if passed symbol table is NULL.
> > > >
> > > > Is this a fix, or a cleanup?
> > >
> > > It is a fix but for a case which normally doesn't happen. It is
> > > triggered by the macro in patch 26 due to the ad-hoc struct output_ctx
> > > definition not populating the symbol tables.
> > 
> > For the debug thing that is unused? ie. #define expr_print_debug(expr)
> 
> Yes, exactly. It basically does:
> 
> | {
> | 	struct output_ctx octx = { .output_fp = stdout };
> | 
> | 	expr_print(expr, &octx);
> | }
> 
> Depending on type of 'expr', this may then call
> symbolic_constant_print(NULL, ...).
> 
> > This cannot ever happen in the code:
> > 
> > src/datatype.c: return symbolic_constant_print(octx->tbl.mark, expr, true, octx);
> > src/meta.c:     return symbolic_constant_print(&pkttype_type_tbl, expr, false, octx);
> > src/meta.c:     return symbolic_constant_print(octx->tbl.devgroup, expr, true, octx);
> > src/meta.c:     return symbolic_constant_print(&day_type_tbl, expr, true, octx);
> > src/proto.c:    return symbolic_constant_print(&ethertype_tbl, expr, false, octx);
> > src/rt.c:       return symbolic_constant_print(octx->tbl.realm, expr, true, octx);
> > 
> > And here:
> > 
> > void datatype_print(const struct expr *expr, struct output_ctx *octx)
> > {
> > ...
> >                 if (dtype->sym_tbl != NULL)
> >                         return symbolic_constant_print(dtype->sym_tbl, expr,
> >                                                        false, octx);
> > 
> > Sorry but this is all sufficiently complex to add misleading hints.
> > 
> > If you need to extend anything, then please have a look at extending
> > --debug to expose more information that you need.
> > 
> > Sorry, I do not see this is an improvement.
> 
> OK, no problem. I can just drop patch 26 and this one from the series.

I inclined to think that this is scaffolding that has helped you to
develop this series, but I am not sure expr_print_debug() will be of
use for me in the future.