Jeremy Sowden <jeremy@xxxxxxxxxx> 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@xxxxxxxxxx> 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".
