On Thu, Nov 27, 2014 at 05:09:06PM +0000, Ola Liljedahl wrote: > On 27 November 2014 at 17:56, Stuart Haslam <[email protected]> wrote: > > Attempts to enq to a pktin queue or deq from a pktout queue are > > programming errors, so abort. > Are there more cases of such programming errors where we should > specify the API semantics to be undefined instead of having to always > detect the error and return some error code? Undefined semantics gives > us the possibility to optimize for performance and not even try to > detect such errors. Default for any ODP implementation should still be > to attempt to detect programming errors and to abort the program > execution (e.g. call ODP_ABORT). The user will get a core dump they > can debug and find the source of their programming errors. > > -- Ola >
Pretty much every API gives the caller an opportunity to screw up by passing invalid handles etc. The semantics of parameter validation aren't defined per API but the architecture documents say this; (from api_guide_lines.dox) -- validations that can be performed statically at compile time or at little to no runtime cost SHOULD be considered, APIs MAY choose to leave behavior as undefined when presented with invalid parameters in the interest of runtime efficiency -- Which makes sense. In terms of what the linux-generic implementation actually does, the majority of the APIs either do no validation or use ODP_ASSERT(). The pktio APIs seem to be the odd-ones-out as most validate parameters and return -1 on error without printing anything, so I think these should be changed to ODP_ASSERT()s. -- Stuart. > > > > Signed-off-by: Stuart Haslam <[email protected]> > > --- > > (This code contribution is provided under the terms of agreement > > LES-LTM-21309) > > > > platform/linux-generic/odp_packet_io.c | 22 ++++++++++++++++++---- > > 1 file changed, 18 insertions(+), 4 deletions(-) > > > > diff --git a/platform/linux-generic/odp_packet_io.c > > b/platform/linux-generic/odp_packet_io.c > > index f527e45..161f2ce 100644 > > --- a/platform/linux-generic/odp_packet_io.c > > +++ b/platform/linux-generic/odp_packet_io.c > > @@ -384,6 +384,9 @@ int pktout_enqueue(queue_entry_t *qentry, > > odp_buffer_hdr_t *buf_hdr) > > odp_buffer_hdr_t *pktout_dequeue(queue_entry_t *qentry) > > { > > (void)qentry; > > + > > + ODP_ABORT("attempted dequeue from a pktout queue"); > > + > > return NULL; > > } > > > > @@ -408,13 +411,19 @@ int pktout_deq_multi(queue_entry_t *qentry, > > odp_buffer_hdr_t *buf_hdr[], > > (void)buf_hdr; > > (void)num; > > > > + ODP_ABORT("attempted dequeue from a pktout queue"); > > + > > return 0; > > } > > > > int pktin_enqueue(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr) > > { > > - /* Use default action */ > > - return queue_enq(qentry, buf_hdr); > > + (void)qentry; > > + (void)buf_hdr; > > + > > + ODP_ABORT("attempted enqueue to a pktin queue"); > > + > > + return -1; > > } > > > > odp_buffer_hdr_t *pktin_dequeue(queue_entry_t *qentry) > > @@ -445,8 +454,13 @@ odp_buffer_hdr_t *pktin_dequeue(queue_entry_t *qentry) > > > > int pktin_enq_multi(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr[], > > int num) > > { > > - /* Use default action */ > > - return queue_enq_multi(qentry, buf_hdr, num); > > + (void)qentry; > > + (void)buf_hdr; > > + (void)num; > > + > > + ODP_ABORT("attempted enqueue to a pktin queue"); > > + > > + return 0; > > } > > > > int pktin_deq_multi(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr[], > > int num) > > -- > > 2.1.1 > > > > > > > > _______________________________________________ > > lng-odp mailing list > > [email protected] > > http://lists.linaro.org/mailman/listinfo/lng-odp > -- Stuart. _______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
