| Message ID | 1436551839-27798-1-git-send-email-zoltan.kiss@linaro.org |
|---|---|
| State | New |
| Headers | show |
Looks good for me. Only description needed to be fixed: On 07/10/15 21:10, Zoltan Kiss wrote: > Unsent packet has to be released. If the event type is obvious from the > context, use directly the relevant release functions, otherwise > odp_event(free). odp_event_free(evt), right? I can fix that on merge. Maxim. > Wider error handling is attempted, but this patch can't fix all the flaws > in the many calling functions of odp_queue_enq() > > Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> > --- > > v5: fix array use in last hunk > > v6: handle negative 'ret' in last hunk > > v7: refactor all hunks to similar than last one > > test/performance/odp_pktio_perf.c | 10 +++++++++- > test/performance/odp_scheduling.c | 8 ++++++-- > test/validation/pktio/pktio.c | 3 +++ > test/validation/queue/queue.c | 4 ++++ > 4 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/test/performance/odp_pktio_perf.c b/test/performance/odp_pktio_perf.c > index f4d9d2d..c2e95fb 100644 > --- a/test/performance/odp_pktio_perf.c > +++ b/test/performance/odp_pktio_perf.c > @@ -270,12 +270,20 @@ static int alloc_packets(odp_event_t *event_tbl, int num_pkts) > static int send_packets(odp_queue_t outq, > odp_event_t *event_tbl, unsigned num_pkts) > { > + int ret; > + unsigned i; > + > if (num_pkts == 0) > return 0; > else if (num_pkts == 1) > return odp_queue_enq(outq, event_tbl[0]) == 0 ? 1 : 0; > > - return odp_queue_enq_multi(outq, event_tbl, num_pkts); > + ret = odp_queue_enq_multi(outq, event_tbl, num_pkts); > + i = ret < 0 ? 0 : ret; > + for ( ; i < num_pkts; i++) > + odp_event_free(event_tbl[i]); > + return ret; > + > } > > /* > diff --git a/test/performance/odp_scheduling.c b/test/performance/odp_scheduling.c > index 99f0f9b..a9ebfaa 100644 > --- a/test/performance/odp_scheduling.c > +++ b/test/performance/odp_scheduling.c > @@ -530,9 +530,13 @@ static int test_schedule_multi(const char *str, int thr, > } > > /* Assume we can enqueue all events */ > - if (odp_queue_enq_multi(queue, ev, MULTI_BUFS_MAX) != > - MULTI_BUFS_MAX) { > + num = odp_queue_enq_multi(queue, ev, MULTI_BUFS_MAX); > + if (num != MULTI_BUFS_MAX) { > LOG_ERR(" [%i] Queue enqueue failed.\n", thr); > + j = num < 0 ? 0 : num; > + for ( ; j < MULTI_BUFS_MAX; j++) > + odp_event_free(ev[j]); > + > return -1; > } > } > diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c > index 82ced5c..283dd92 100644 > --- a/test/validation/pktio/pktio.c > +++ b/test/validation/pktio/pktio.c > @@ -388,6 +388,9 @@ static void pktio_txrx_multi(pktio_info_t *pktio_a, pktio_info_t *pktio_b, > ret = odp_queue_enq_multi(pktio_a->outq, tx_ev, num_pkts); > if (ret != num_pkts) { > CU_FAIL("failed to enqueue test packets"); > + i = ret < 0 ? 0 : ret; > + for ( ; i < num_pkts; i++) > + odp_packet_free(tx_ev[i]); > return; > } > } > diff --git a/test/validation/queue/queue.c b/test/validation/queue/queue.c > index f686b71..96dc932 100644 > --- a/test/validation/queue/queue.c > +++ b/test/validation/queue/queue.c > @@ -88,6 +88,10 @@ static void queue_test_sunnydays(void) > */ > ret = odp_queue_enq_multi(queue_id, enev, MAX_BUFFER_QUEUE); > CU_ASSERT(MAX_BUFFER_QUEUE == ret); > + i = ret < 0 ? 0 : ret; > + for ( ; i < MAX_BUFFER_QUEUE; i++) > + odp_event_free(enev[i]); > + > pev_tmp = deev; > do { > deq_ret = odp_queue_deq_multi(queue_id, pev_tmp,
On 13/07/15 11:29, Maxim Uvarov wrote: > Looks good for me. Only description needed to be fixed: > > > > On 07/10/15 21:10, Zoltan Kiss wrote: >> Unsent packet has to be released. If the event type is obvious from the >> context, use directly the relevant release functions, otherwise >> odp_event(free). > odp_event_free(evt), right? I can fix that on merge. > > Maxim. Yes, please do
pktio.c: In function ‘pktio_txrx_multi’:
pktio.c:393:5: error: passing argument 1 of ‘odp_packet_free’ from
incompatible pointer type [-Werror]
odp_packet_free(tx_ev[i]);
^
In file included from
../../../platform/linux-generic/include/odp/packet.h:35:0,
from ../../../include/odp.h:47,
from pktio.c:6:
../../../include/odp/api/packet.h:100:6: note: expected ‘odp_packet_t’
but argument is of type ‘odp_event_t’
void odp_packet_free(odp_packet_t pkt);
On 07/13/15 15:14, Zoltan Kiss wrote:
>
>
> On 13/07/15 11:29, Maxim Uvarov wrote:
>> Looks good for me. Only description needed to be fixed:
>>
>>
>>
>> On 07/10/15 21:10, Zoltan Kiss wrote:
>>> Unsent packet has to be released. If the event type is obvious from the
>>> context, use directly the relevant release functions, otherwise
>>> odp_event(free).
>> odp_event_free(evt), right? I can fix that on merge.
>>
>> Maxim.
>
> Yes, please do
"Oops!... I Did It Again" (Max Martin / Rami Yacoub) It was correct originally, but I missed it with the recent rework, I'll send it again. On 13/07/15 14:04, Maxim Uvarov wrote: > pktio.c: In function ‘pktio_txrx_multi’: > pktio.c:393:5: error: passing argument 1 of ‘odp_packet_free’ from > incompatible pointer type [-Werror] > odp_packet_free(tx_ev[i]); > ^ > In file included from > ../../../platform/linux-generic/include/odp/packet.h:35:0, > from ../../../include/odp.h:47, > from pktio.c:6: > ../../../include/odp/api/packet.h:100:6: note: expected ‘odp_packet_t’ > but argument is of type ‘odp_event_t’ > void odp_packet_free(odp_packet_t pkt); > > On 07/13/15 15:14, Zoltan Kiss wrote: >> >> >> On 13/07/15 11:29, Maxim Uvarov wrote: >>> Looks good for me. Only description needed to be fixed: >>> >>> >>> >>> On 07/10/15 21:10, Zoltan Kiss wrote: >>>> Unsent packet has to be released. If the event type is obvious from the >>>> context, use directly the relevant release functions, otherwise >>>> odp_event(free). >>> odp_event_free(evt), right? I can fix that on merge. >>> >>> Maxim. >> >> Yes, please do >
diff --git a/test/performance/odp_pktio_perf.c b/test/performance/odp_pktio_perf.c index f4d9d2d..c2e95fb 100644 --- a/test/performance/odp_pktio_perf.c +++ b/test/performance/odp_pktio_perf.c @@ -270,12 +270,20 @@ static int alloc_packets(odp_event_t *event_tbl, int num_pkts) static int send_packets(odp_queue_t outq, odp_event_t *event_tbl, unsigned num_pkts) { + int ret; + unsigned i; + if (num_pkts == 0) return 0; else if (num_pkts == 1) return odp_queue_enq(outq, event_tbl[0]) == 0 ? 1 : 0; - return odp_queue_enq_multi(outq, event_tbl, num_pkts); + ret = odp_queue_enq_multi(outq, event_tbl, num_pkts); + i = ret < 0 ? 0 : ret; + for ( ; i < num_pkts; i++) + odp_event_free(event_tbl[i]); + return ret; + } /* diff --git a/test/performance/odp_scheduling.c b/test/performance/odp_scheduling.c index 99f0f9b..a9ebfaa 100644 --- a/test/performance/odp_scheduling.c +++ b/test/performance/odp_scheduling.c @@ -530,9 +530,13 @@ static int test_schedule_multi(const char *str, int thr, } /* Assume we can enqueue all events */ - if (odp_queue_enq_multi(queue, ev, MULTI_BUFS_MAX) != - MULTI_BUFS_MAX) { + num = odp_queue_enq_multi(queue, ev, MULTI_BUFS_MAX); + if (num != MULTI_BUFS_MAX) { LOG_ERR(" [%i] Queue enqueue failed.\n", thr); + j = num < 0 ? 0 : num; + for ( ; j < MULTI_BUFS_MAX; j++) + odp_event_free(ev[j]); + return -1; } } diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c index 82ced5c..283dd92 100644 --- a/test/validation/pktio/pktio.c +++ b/test/validation/pktio/pktio.c @@ -388,6 +388,9 @@ static void pktio_txrx_multi(pktio_info_t *pktio_a, pktio_info_t *pktio_b, ret = odp_queue_enq_multi(pktio_a->outq, tx_ev, num_pkts); if (ret != num_pkts) { CU_FAIL("failed to enqueue test packets"); + i = ret < 0 ? 0 : ret; + for ( ; i < num_pkts; i++) + odp_packet_free(tx_ev[i]); return; } } diff --git a/test/validation/queue/queue.c b/test/validation/queue/queue.c index f686b71..96dc932 100644 --- a/test/validation/queue/queue.c +++ b/test/validation/queue/queue.c @@ -88,6 +88,10 @@ static void queue_test_sunnydays(void) */ ret = odp_queue_enq_multi(queue_id, enev, MAX_BUFFER_QUEUE); CU_ASSERT(MAX_BUFFER_QUEUE == ret); + i = ret < 0 ? 0 : ret; + for ( ; i < MAX_BUFFER_QUEUE; i++) + odp_event_free(enev[i]); + pev_tmp = deev; do { deq_ret = odp_queue_deq_multi(queue_id, pev_tmp,
Unsent packet has to be released. If the event type is obvious from the context, use directly the relevant release functions, otherwise odp_event(free). Wider error handling is attempted, but this patch can't fix all the flaws in the many calling functions of odp_queue_enq() Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> --- v5: fix array use in last hunk v6: handle negative 'ret' in last hunk v7: refactor all hunks to similar than last one test/performance/odp_pktio_perf.c | 10 +++++++++- test/performance/odp_scheduling.c | 8 ++++++-- test/validation/pktio/pktio.c | 3 +++ test/validation/queue/queue.c | 4 ++++ 4 files changed, 22 insertions(+), 3 deletions(-)