Re: [nft PATCH 3/4] Support 'add/insert rule index <IDX>'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Pablo,

On Wed, May 09, 2018 at 06:42:17PM +0200, Pablo Neira Ayuso wrote:
> I just noticed one thing, see below.
> 
> On Wed, May 09, 2018 at 04:03:42PM +0200, Phil Sutter wrote:
> > diff --git a/src/evaluate.c b/src/evaluate.c
> > index 46c97606ea8af..cb27f7c269049 100644
> > --- a/src/evaluate.c
> > +++ b/src/evaluate.c
> > @@ -2851,6 +2851,47 @@ static int flowtable_evaluate(struct eval_ctx *ctx, struct flowtable *ft)
> >  	return 0;
> >  }
> >  
> > +/* Convert rule's handle.index into handle.position. */
> > +static int rule_translate_index(struct eval_ctx *ctx, struct rule *rule)
> > +{
[...]
> > +	if (!rule->handle.position.id)
> > +		return cmd_error(ctx, &rule->handle.index.location,
> > +				"Could not process rule: %s",
> > +				strerror(EINVAL));
> 
> I think this should be ENOENT too.
> 
> We should use EINVAL only for netlink, when attributes are missing.
> 
> EINVAL is quite overloaded already, so better avoid this unspecific
> error type.

Yes, that's true. I used EINVAL because if no rule was found, the index
given was too large regarding the number of rules already in the chain
and therefore the index is invalid. But no big deal, I'll follow-up with
a patch changing it to ENOENT.

Thanks, Phil
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux