On Mon 28 May 2018 at 21:31, Marcelo Ricardo Leitner
<marcelo.leit...@gmail.com> wrote:
> On Mon, May 28, 2018 at 12:17:29AM +0300, Vlad Buslov wrote:
> ...
>> -int tcf_action_destroy(struct list_head *actions, int bind)
>> +int tcf_action_destroy(struct tc_action *actions[], int bind)
>> {
>> const struct tc_action_ops *ops;
>> - struct tc_action *a, *tmp;
>> - int ret = 0;
>> + struct tc_action *a;
>> + int ret = 0, i;
>>
>> - list_for_each_entry_safe(a, tmp, actions, list) {
>> + for (i = 0; i < TCA_ACT_MAX_PRIO && actions[i]; i++) {
> ...
>> @@ -878,10 +881,9 @@ struct tc_action *tcf_action_init_1(struct net *net,
>> struct tcf_proto *tp,
>> if (TC_ACT_EXT_CMP(a->tcfa_action, TC_ACT_GOTO_CHAIN)) {
>> err = tcf_action_goto_chain_init(a, tp);
>> if (err) {
>> - LIST_HEAD(actions);
>> + struct tc_action *actions[TCA_ACT_MAX_PRIO] = { a };
>
> Somewhat nit.. Considering tcf_action_destroy will stop at the first
> NULL, you need only 2 slots here.
Yes, I guess NULLing whole array when only first pointer is used, is
redundant. I didn't want to be too clever in this patch and made all
actions array of same size, but I don't see anything potentially
dangerous in reducing this one.
>
>>
>> - list_add_tail(&a->list, &actions);
>> - tcf_action_destroy(&actions, bind);
>> + tcf_action_destroy(actions, bind);
>> NL_SET_ERR_MSG(extack, "Failed to init TC action
>> chain");
>> return ERR_PTR(err);
>> }
Thank you for reviewing my code!
