On 03/12/15 15:02, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----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.
I see. Maybe it would worth to add that ENOMEM-like cases are not
failures in this case, because sometimes it is interpreted as 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)
Makes sense
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
