> -----Original Message-----
> From: lng-odp [mailto:[email protected]] On Behalf Of EXT
> Zoltan Kiss
> Sent: Thursday, December 03, 2015 4:30 PM
> To: Nicolas Morey-Chaisemartin; [email protected]
> Subject: Re: [lng-odp] [API-NEXTv6 1/7] api: buffer: add functions to
> alloc/free multiple buffers at once
>
> Hi,
>
> I know it's a late cry, but I've found two problems while implementing
> this for ODP-DPDK, and the apply for odp_packet_alloc_multi() as well.
> See inline:
>
> On 28/10/15 15:31, Nicolas Morey-Chaisemartin wrote:
> > Signed-off-by: Nicolas Morey-Chaisemartin <[email protected]>
> > Reviewed-by: Petri Savolainen <[email protected]>
> > ---
> > include/odp/api/buffer.h | 26 ++++++++++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/include/odp/api/buffer.h b/include/odp/api/buffer.h
> > index aea273f..6631f47 100644
> > --- a/include/odp/api/buffer.h
> > +++ b/include/odp/api/buffer.h
> > @@ -105,6 +105,20 @@ odp_pool_t odp_buffer_pool(odp_buffer_t buf);
> > odp_buffer_t odp_buffer_alloc(odp_pool_t pool);
> >
> > /**
> > + * Allocate multiple buffers
> > +
> > + * Otherwise like odp_buffer_alloc(), but allocates multiple buffers
> from a pool
> > + *
> > + * @param pool Pool handle
> > + * @param[out] buf Array of buffer handles for output
> > + * @param num Maximum number of buffers to allocate
> > + *
> > + * @return Number of buffers actually allocated (0 ... num)
> > + * @retval <0 on failure
>
> These two lines contradicts each other: the first says we should return
> how much we managed to allocate, and we will always allocate at least 0
> buffers.
> The second says in case of error a negative value, but even if we fail
> straight away, the first line commands us to return 0. Moreover, if we
> managed to allocate at least some of the buffers and then we return a
> negative value due to some error (e.g. ENOMEM, or anything else), the
> application won't know if there is any buffer to use in the array.
> Either we should clarify that the platform should release them in case
> of an error, or (and I think that would be the better thing to do) we
> should return how much we allocated.
> Altogether, I think we should remove the "<0 on failure" statement and
> set the errno instead. If you agree, Nicolas, would you submit a patch
> for that?
>
This is inline with other _multi() calls. 0 packets/buffers/events processed is
a valid outcome. Pool was empty, queue was full, etc. Negative is a real
failure: bad handle, etc. In case of failure, the call must not perform the
action (e.g. half way). Either 0...num packets were processed, or nothing was
processed in case of failure.
> > + */
> > +int odp_buffer_alloc_multi(odp_pool_t pool, odp_buffer_t buf[], int
> num);
>
> 'num' should be unsigned, a negative number doesn't make sense in this
> case. Although changing this breaks the ABI, so I'm not sure if we
> should fix this or just leave it.
This is inline with other multi calls. Num is int since return value is int.
User can compare (without warnings)
if (odp_buffer_alloc_multi(...., num) == num)
// Success...
-Petri
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
> https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
