> -----Original Message-----
> From: lng-odp-boun...@lists.linaro.org [mailto:lng-odp-
> boun...@lists.linaro.org] On Behalf Of ext Ola Liljedahl
> Sent: Wednesday, January 28, 2015 7:06 PM
> To: LNG ODP Mailman List
> Subject: [lng-odp] API questions - sizes and numelems as inputs and
> outputs
>
> 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.
How use full the function is if it does not fill the whole buffer with rand
data? I think in this case should be success vs. failure. In success the whole
buffer is written.
>
> /**
> * 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)).
What is a successful copy of a zero length MAC address? I think it's OK to
return failure (==0), if user asks for a copy of MAC address from a interface
that do not have one.
>
> /**
> * 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.
It's better to keep this simple: success or failure. In success "len" bytes was
copied. It's not important how many bytes were copied in failure.
Failures:
offset > odp_packet_len()
offset + len > odp_packet_len()
>
> /**
> * 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?
Failures:
offset > odp_packet_len()
offset + len > odp_packet_len()
>
> /**
> * 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?
This is why we are going through the APIs and check/harmonize return values ...
the function definition is from Nov 13.
Int is good return type here. Should we use "int len" param to match the return
value?
int i = 0;
int len = 10;
while (len) {
int sent = odp_pktio_send(id, &pkt[i], len);
if (sent < 0)
ABORT();
i += sent;
len -= sent;
}
This would generate compiler warning if mixing unsigned and int:
unsigned len = 10;
int sent = odp_pktio_send(id, &pkt[i], len);
if (sent != len)
LOG("could not send everything at once");
>
> /**
> * 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);
True. It's reasonable to allow incremental usage.
@return Number of enqueued events on success.
>
> /**
> * 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?
True. Currently no failure specified, so it could return unsigned. However, to
harmonize and to enable forward-compatible failure case: return int / int num?
>
> 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.
I think, this 2) should be avoided for now and specify CONFIG_XXX_MAX macros
instead. That avoids "int *len" output parameter or complex return value ranges
(len for failure).
>
> 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.
This has two sub classes:
1) 0 consumed/produced is an error
- Resource is full. User have to do something about it before a re-try may
succeed.
2) 0 is acceptable
- Resource is currently busy, but re-try can succeed later on (without user
doing anything else in between)
>
> 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
"Required size" not needed feature => output param not needed.
ssize_t is problematic with sizeof().
# if cannot fail or zero can indicate failure
size_t func(void* buf, size_t len)
# if can fail and zero cannot be the failure
ssize_t func(void* buf, size_t len)
>
> 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)
# This is OK. We just have to spec if 0 is success or failure.
int func(hdl, array[], int num_elems_in_array)
-Petri
>
> 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
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
