Talking about API's that take some type of element count or buffer
size as input and return a value related to that. There seems to be a
number of different models.
/**
* Generate random byte string
*
* @param buf Pointer to store result
* @param len Pointer to input length value as well as return value
I assume the "return value" is the number of result bytes stored. But
this is not very clear.
* @param use_entropy Use entropy
*
* @todo Define the implication of the use_entropy parameter
*
* @retval 0 on success
Not the same "return value" as described above.
* @retval <0 on failure
*/
int odp_hw_random_get(uint8_t *buf, size_t *len, bool use_entropy);
Here we return the actual size written (I assume) through the len
parameter which is consequently passed by reference.
/**
* Write CPU mask as a string of hexadecimal digits
*
* @param mask CPU mask to write
* @param buf Output buffer
* @param bufsz Size of output buffer
*
* @return >0 on success. Number of characters written (including the
* terminating null char).
* @return 0 on failure (output buffer too small).
*/
size_t odp_cpumask_to_str(const odp_cpumask_t *mask, char *buf, size_t bufsz);
The new definition of odp_cpumask_to_str() as recently agreed. Number
of characters written is returned. It can't be 0 so 0 can be used as
an error indicator. In other formatting-type of functions, 0 might be
a valid return value (it primarily depends on whether the terminating
null char should be included in the count).
* @param[out] mac_addr Storage for MAC address of the packet IO interface.
* @param addr_size Storage size for the address
*
* @retval Number of bytes written on success, 0 on failure.
*/
size_t odp_pktio_mac_addr(odp_pktio_t id, void *mac_addr, size_t addr_size);
This function returns 0 for failure (but this is incidental and
prevents us to support pktio types that have zero-length MAC address
(i.e. does not have any MAC address)).
/**
* Copy data from packet
*
* Copy 'len' bytes of data from the packet level offset to the destination
* address.
*
* @param pkt Packet handle
* @param offset Byte offset into the packet
* @param len Number of bytes to copy
* @param dst Destination address
*
* @retval 0 on success
* @retval <0 on failure
Not documented what constitutes a failure.
*/
int odp_packet_copydata_out(odp_packet_t pkt, uint32_t offset,
uint32_t len, void *dst);
What if less than len bytes could be copied (because reaching end of
packet)? How is this reported? Should we use "uint32_t *len" and
update it on return with the actual number of bytes copied? Or is this
situation considered an error and reported with a negative return
value? Seems excessive. Better to just specify len as the size of the
output buffer and in some way be notified about the actual number of
bytes copied.
/**
* Copy data into packet
*
* Copy 'len' bytes of data from the source address into the packet level
* offset. Maximum number of bytes to copy is packet data length minus the
* offset. Packet is not modified on an error.
*
* @param pkt Packet handle
* @param offset Byte offset into the packet
* @param len Number of bytes to copy
* @param src Source address
*
* @retval 0 on success
* @retval <0 on failure
Not documented what constitutes a failure.
*/
int odp_packet_copydata_in(odp_packet_t pkt, uint32_t offset,
uint32_t len, const void *src);
Similar for this function. Is it an error if 'len' bytes does not fit
into the current packet size? Is this good semantics?
/**
* Receive packets
*
* @param id ODP packet IO handle
* @param pkt_table[] Storage for received packets (filled by function)
* @param len Length of pkt_table[], i.e. max number of pkts to receive
*
* @return Number of packets received
* @retval <0 on failure
*/
int odp_pktio_recv(odp_pktio_t id, odp_packet_t pkt_table[], unsigned len);
int odp_pktio_send(odp_pktio_t id, odp_packet_t pkt_table[], unsigned len);
Here is a classic. Attempt to receive up to N items. Return actual
number of items received or <0 on failure. 0 is a valid success code
(not a failure). Why can't this be our model?
The prototype uses unsigned and signed ints. How is different from
using size_t and ssize_t?
/**
* Enqueue multiple events to a queue
*
* @param queue Queue handle
* @param ev Event handles
* @param num Number of event handles
*
* @retval 0 on success
* @retval <0 on failure
*/
int odp_queue_enq_multi(odp_queue_t queue, odp_event_t ev[], int num);
Here we use "int" for the number of items, at least this matches with
the return type. But there is no indication that the function returns
the number of events that were enqueued. What happens if we are trying
to enqueue events to a fixed-size queue (e.g. ring buffer) and all
items do not fit? Is this considered a failure? It could be
problematic to have to retry this call until it is possible to enqueue
all items in the same go. Incremental success should be allowed for
unless there is a good explanation why this "atomic" semantics is
desired. The application can decide what to do with those events that
are not enqueued.
Same for this:
int odp_queue_deq_multi(odp_queue_t queue, odp_event_t events[], int num);
/**
* Schedule multiple events
*
* Like odp_schedule(), but returns multiple events from a queue.
*
* @param from Output parameter for the source queue (where the event was
* dequeued from). Ignored if NULL.
* @param wait Minimum time to wait for an event. Waits infinitely, if set to
* ODP_SCHED_WAIT. Does not wait, if set to ODP_SCHED_NO_WAIT.
* Use odp_schedule_wait_time() to convert time to other wait
* values.
* @param events Event array for output
* @param num Maximum number of events to output
*
* @return Number of events outputed (0 ... num)
Number of events scheduled and stored in the events array?
*/
int odp_schedule_multi(odp_queue_t *from, uint64_t wait, odp_event_t events[],
unsigned int num);
Unsigned int for number of items, int used as return type (number of
items processed) but no errors are supposed to occur so negative range
is not used. Why then use int and not unsigned int for return type?
So a lot of different models for basically two cases.
Case 1: Function writes data (size possibly not known in advance) to a
buffer. Caller must specify buffer size. Function must return number
of bytes actually written. And be able to return 1) output buffer too
small and 2) minimum required size of output buffer for successful
operation.
Case 2: Caller specifies number of items for function to consume or
produce. Possibly that many items cannot be consumed (e.g. not room
enough in some ring buffer) or cannot be produced (not available at
source). Not all items have to be consumed or produced for success
(incremental success is allowed), indeed even 0 items consumed or
produced is a success, no error has occurred. Function must be able to
return number of items consumed/produced. Some functions must also be
able to return errors.
For #1 I suggest:
ssize_t func(void *buf, ssize_t *bufsize);
@return >= 0 on success, number of bytes written (0 is OK when applicable)
@return <= on failure, if buffer too small (need specific return code
to indicate this?) then "*bufsize" is updated with the required size
Alternatively if the function is not working with bytes or chars but
some other type of object, use "int" instead of "ssize_t".
For #2 I suggest:
int func(odp_handle_t hdl, /const/ odp_object_t array[], int
num_elems_in_array);
@return >= 0 on success, number of elements consumed or produced (0 is
normally OK)
@return <0 on failure (when applicable, some functions may not
actually need to report failures but we can keep this for consistency)
Could there be exceptions to these design rules? Some function that
might need to operate on >=half the memory space as a consecutive
area? Not going to happen for 64-bit architectures. Plausible for
32-bit architectures but how likely in practice?
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
