Re: [nft PATCH 08/11] src: add a helper that returns a payload dependency for a particular base

[Florian Westphal <fw@strlen.de>]

Jeremy Sowden <jeremy@azazel.net> wrote:
> On 2022-01-15, at 16:57:07 +0000, Jeremy Sowden wrote:
> > On 2022-01-15, at 17:48:24 +0100, Florian Westphal wrote:
> > > Jeremy Sowden <jeremy@azazel.net> wrote:
> > > > Currently, with only one base and dependency stored this is
> > > > superfluous, but it will become more useful when the next commit
> > > > adds support for storing a payload for every base.
> > >
> > > > +	dep = payload_dependency_get(ctx, PROTO_BASE_NETWORK_HDR)->expr;
> > >
> > > This new helper can return NULL, would you mind reworking this to
> > > add error checks here?
> >
> > Yup.
> 
> Actually, let me provide a bit more context:
> 
>   @@ -2060,11 +2060,13 @@ static bool meta_may_dependency_kill(struct payload_dep_ctx *ctx,
>                                        const struct expr *expr)
>    {
>           uint8_t l4proto, nfproto = NFPROTO_UNSPEC;
>   -       struct expr *dep = ctx->pdep->expr;
>   +       struct expr *dep;
> 
>           if (ctx->pbase != PROTO_BASE_NETWORK_HDR)
>                   return true;
> 
>   +       dep = payload_dependency_get(ctx, PROTO_BASE_NETWORK_HDR)->expr;
>   +
> 
> We check that there is a PROTO_BASE_NETWORK_HDR dependency immediately
> before calling the helper.

Perhaps remove the check?

bla()->foo looks weird, esp. given bla() last line is "return NULL".