OpenFastPath / ofp

OpenFastPath project
BSD 3-Clause "New" or "Revised" License
349 stars 126 forks source link

Ofp timer leak #234

Closed HsuJv closed 4 years ago

HsuJv commented 4 years ago

Hi there,

Do you think it's necessary to free the timer after set_abs failed?

src/ofp_timer.c:  417
        timer = ofp_timer_alloc(cpu_id, bufdata);

        if (timer == ODP_TIMER_INVALID) {
            odp_timeout_free(tmo);
            odp_buffer_free(buf);
            OFP_ERR("odp_timer_alloc failed");
            return ODP_TIMER_INVALID;
        }

        t = odp_timer_set_abs(timer, tick, &bufdata->t_ev);

        if (t != ODP_TIMER_SUCCESS) {
            odp_timeout_free(tmo);
            odp_buffer_free(buf);
=======================>odp_timer_free(timer);
            OFP_ERR("odp_timer_set_abs failed");
            return ODP_TIMER_INVALID;
        }

see the line that arrowed.

Anyone could give an answer? I can't figure out where the timer to be freed if odp_timer_set_abs returns any value rather than ODP_TIMER_SUCCESS

@matiaselo @JereLeppanen

Regards.

bogdanPricope commented 4 years ago

Looks OK. Maybe put it first in the scope (reverse order of allocation, etc.). e.g. if (t != ODP_TIMER_SUCCESS) { odp_timer_free(timer); odp_timeout_free(tmo); odp_buffer_free(buf); OFP_ERR("odp_timer_set_abs failed"); return ODP_TIMER_INVALID; }