hello Cong, thanks for reviewing.
On Wed, 2019-02-27 at 17:28 -0800, Cong Wang wrote:
> On Wed, Feb 27, 2019 at 9:41 AM Davide Caratti <dcara...@redhat.com> wrote:
> > +int tcf_action_check_ctrlact(int action, struct tcf_proto *tp,
> > + struct tcf_chain **handle,
>
> Please use a better name than 'handle'. 'handle' is usually used
> for a hex numeric ID. Here you just want to save the allocated
> tcf_chain to this address.
ok, understood. I will use 'newchain', like done elsewhere.
> + *handle = tcf_chain_get_by_act(tp->chain->block, chain_index);
> > + if (!*handle) {
> > + ret = -ENOMEM;
>
> Is -ENOMEM okay here? I feel like it should be -ENOSPC or whatever
> tcf_chain_get_by_act() says.
tcf_chain_by_act() calls __tcf_chain_get() with 'create' equal to true.
So, if a chain with the given index does not exist, tcf_chain_create()
will try to create it - and there, the only possible failure I see is
kzalloc().
That's why I think -ENOMEM is ok, at least for the current net/sched code.
--
davide
