On 16/01/2015 19:12, "Ola Liljedahl" <[email protected]> wrote:
>On 16 January 2015 at 16:44, Mario Torrecillas Rodriguez ><[email protected]> wrote: >> Added odp_errno.c and odp_errno.h >> Changed odp_packet_io and odp_timer to use it. >> >> Signed-off-by: Mario Torrecillas Rodriguez >><[email protected]> >> --- >> (This code contribution is provided under the terms of agreement >>LES-LTM-21309) >> >> platform/linux-generic/Makefile.am | 2 + >> platform/linux-generic/include/api/odp_errno.h | 62 >>++++++++++++++++++++++++++ >> platform/linux-generic/odp_errno.c | 42 +++++++++++++++++ >> platform/linux-generic/odp_packet_io.c | 4 +- >> platform/linux-generic/odp_packet_socket.c | 16 +++++++ >> platform/linux-generic/odp_timer.c | 5 ++- >> 6 files changed, 127 insertions(+), 4 deletions(-) >> create mode 100644 platform/linux-generic/include/api/odp_errno.h >> create mode 100644 platform/linux-generic/odp_errno.c >> >> diff --git a/platform/linux-generic/Makefile.am >>b/platform/linux-generic/Makefile.am >> index 4535c57..3795968 100644 >> --- a/platform/linux-generic/Makefile.am >> +++ b/platform/linux-generic/Makefile.am >> @@ -8,6 +8,7 @@ AM_CFLAGS += -I$(top_srcdir)/helper/include >> include_HEADERS = \ >> >>$(top_srcdir)/platform/linux-generic/include/api/odp.h \ >> >>$(top_srcdir)/platform/linux-generic/include/api/odp_align.h \ >> + >>$(top_srcdir)/platform/linux-generic/include/api/odp_errno.h \ >Should files be listed in alphabetical order? > >> >>$(top_srcdir)/platform/linux-generic/include/api/odp_atomic.h \ >> >>$(top_srcdir)/platform/linux-generic/include/api/odp_barrier.h \ >> >>$(top_srcdir)/platform/linux-generic/include/api/odp_buffer.h \ >> @@ -75,6 +76,7 @@ subdirheaders_HEADERS = \ >> >> __LIB__libodp_la_SOURCES = \ >> odp_barrier.c \ >> + odp_errno.c \ >Order? > >> odp_buffer.c \ >> odp_buffer_pool.c \ >> odp_classification.c \ >> diff --git a/platform/linux-generic/include/api/odp_errno.h >>b/platform/linux-generic/include/api/odp_errno.h >> new file mode 100644 >> index 0000000..df8acb3 >> --- /dev/null >> +++ b/platform/linux-generic/include/api/odp_errno.h >> @@ -0,0 +1,62 @@ >> +/* Copyright (c) 2015, Linaro Limited >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> + >> +/** >> + * @file >> + * >> + * ODP errno API >> + */ >> + >> +#ifndef ODP_ERRNO_H_ >> +#define ODP_ERRNO_H_ >> + >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> + >> +#include <errno.h> >> + >> +extern __thread int __odp_errno; >Do we need this declaration in this public header file? Is this part >of the public API? Not really, only ODP implementation files should have access to it. Perhaps I should simply create odp_errno_internal.h and have this variable declared there (the file would not contain anything elseŠ). Then ODP libraries would include the internal header file, and ODP applications would include the public one. > >> + >> +/** >> +* Return latest ODP errno >> +* >> +* @retval 0 == no error >> +*/ >> +int odp_errno(void); >> + >> +/** >> +* Set ODP errno to zero >> +*/ >> +void odp_errno_zero(void); >Why do we need this function when we have __odp_errno public? > >Either use an lvalue macro or variable like __odp_errno which can be >referenced directly. >Or have odp_errno() and odp_errno_set(v) functions. >Not a mix of both styles. Applications should be able to clear the variable, but not to change it to an arbitrary value. That¹s the reason odp_errno_zero() exists but odp_errno_set(v) does not. But then again, as you pointed out above, the variable should not be in the public header file. > >> + >> +/** >> +* Print ODP errno >> +* >> +* Interprets the value of ODP errno as an error message, and prints it, >> +* optionally preceding it with the custom message specified in str. >> +* >> +* @param str Pointer to the string to be appended >If this custom message is optional, can you specify NULL here? > >> +*/ >> +void odp_errno_print(const char *str); >> + >> +/** >> +* Error message string >> +* >> +* Interprets the value of ODP errno, generating a string with a >Perhaps document that it uses the system definition of errno. > >> +* message that describes the error. >> +* >> +* @param errnum Error code >> +* >> +* @retval Pointer to the string >> +*/ >> +const char *odp_errno_str(int errnum); >> + >> +#ifdef __cplusplus >> +} >> +#endif >> + >> +#endif >> diff --git a/platform/linux-generic/odp_errno.c >>b/platform/linux-generic/odp_errno.c >> new file mode 100644 >> index 0000000..b0ddfef >> --- /dev/null >> +++ b/platform/linux-generic/odp_errno.c >> @@ -0,0 +1,42 @@ >> +/* Copyright (c) 2015, Linaro Limited >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> + >> +#include <odp_errno.h> >> +#include <string.h> >> +#include <stdio.h> >> + >> +#define ODP_MAX_ERRNO_STR 200 >> + >> +__thread int __odp_errno; >> + >> +int odp_errno(void) >> +{ >> + return __odp_errno; >> +} >> + >> +void odp_errno_zero(void) >> +{ >> + __odp_errno = 0; >> +} >> + >> +void odp_errno_print(const char *str) >> +{ >> + char prefix_str[ODP_MAX_ERRNO_STR]; >> + int n; >> + >> + n = snprintf(prefix_str, ODP_MAX_ERRNO_STR, "%s %s", >> + str, strerror(__odp_errno)); >Why pre-format the string here and risk that it does not fit into the >buffer? Why not call printf directly? Good point. This function was originally implemented differently (using perror()) until I realised it would not work. I switched to printf afterwards, and forgot to remove the pre-formatting. > >> + >> + if (n > 0 && n < ODP_MAX_ERRNO_STR) >> + printf("%s\n", prefix_str); >> + else >> + printf("%s\n", strerror(__odp_errno)); >> +} >> + >> +const char *odp_errno_str(int errnum) >> +{ >> + return strerror(errnum); >> +} >> diff --git a/platform/linux-generic/odp_packet_io.c >>b/platform/linux-generic/odp_packet_io.c >> index cd109d2..c1c79d4 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -18,12 +18,12 @@ >> #include <odp_schedule_internal.h> >> #include <odp_classification_internal.h> >> #include <odp_debug_internal.h> >> +#include <odp_errno.h> >> >> #include <string.h> >> #include <sys/ioctl.h> >> #include <linux/if_arp.h> >> #include <ifaddrs.h> >> -#include <errno.h> >> >> static pktio_table_t *pktio_tbl; >> >> @@ -259,7 +259,7 @@ odp_pktio_t odp_pktio_open(const char *dev, >>odp_buffer_pool_t pool) >> id = odp_pktio_lookup(dev); >> if (id != ODP_PKTIO_INVALID) { >> /* interface is already open */ >> - errno = -EEXIST; >> + __odp_errno = -EEXIST; >> return ODP_PKTIO_INVALID; >> } >> >> diff --git a/platform/linux-generic/odp_packet_socket.c >>b/platform/linux-generic/odp_packet_socket.c >> index da7fb2c..12aa380 100644 >> --- a/platform/linux-generic/odp_packet_socket.c >> +++ b/platform/linux-generic/odp_packet_socket.c >> @@ -40,6 +40,7 @@ >> #include <odp_align_internal.h> >> #include <odp_debug_internal.h> >> #include <odp_hints.h> >> +#include <odp_errno.h> >> >> #include <odph_eth.h> >> #include <odph_ip.h> >> @@ -112,6 +113,7 @@ static int set_pkt_sock_fanout_mmap(pkt_sock_mmap_t >>*const pkt_sock, >> >> err = setsockopt(sockfd, SOL_PACKET, PACKET_FANOUT, &val, >>sizeof(val)); >> if (err != 0) { >> + __odp_errno = errno; >> ODP_ERR("setsockopt(PACKET_FANOUT): %s\n", >>strerror(errno)); >> return -1; >> } >> @@ -185,6 +187,8 @@ int setup_pkt_sock(pkt_sock_t *const pkt_sock, >>const char *netdev, >> return sockfd; >> >> error: >> + __odp_errno = errno; >> + >> return -1; >> } >> >> @@ -195,6 +199,7 @@ error: >> int close_pkt_sock(pkt_sock_t *const pkt_sock) >> { >> if (pkt_sock->sockfd != -1 && close(pkt_sock->sockfd) != 0) { >> + __odp_errno = errno; >> ODP_ERR("close(sockfd): %s\n", strerror(errno)); >> return -1; >> } >> @@ -430,12 +435,14 @@ static int mmap_pkt_socket(void) >> >> int ret, sock = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL)); >> if (sock == -1) { >> + __odp_errno = errno; >> ODP_ERR("socket(SOCK_RAW): %s\n", strerror(errno)); >> return -1; >> } >> >> ret = setsockopt(sock, SOL_PACKET, PACKET_VERSION, &ver, >>sizeof(ver)); >> if (ret == -1) { >> + __odp_errno = errno; >> ODP_ERR("setsockopt(PACKET_VERSION): %s\n", >>strerror(errno)); >> close(sock); >> return -1; >> @@ -568,6 +575,7 @@ static inline unsigned pkt_mmap_v2_tx(int sock, >>struct ring *ring, >> ret = sendto(sock, NULL, 0, MSG_DONTWAIT, NULL, 0); >> if (ret == -1) { >> if (errno != EAGAIN) { >> + __odp_errno = errno; >> ODP_ERR("sendto(pkt mmap): %s\n", >>strerror(errno)); >> return -1; >> } >> @@ -597,6 +605,7 @@ static int mmap_set_packet_loss_discard(int sock) >> ret = setsockopt(sock, SOL_PACKET, PACKET_LOSS, (void >>*)&discard, >> sizeof(discard)); >> if (ret == -1) { >> + __odp_errno = errno; >> ODP_ERR("setsockopt(PACKET_LOSS): %s\n", >>strerror(errno)); >> return -1; >> } >> @@ -623,6 +632,7 @@ static int mmap_setup_ring(int sock, struct ring >>*ring, int type) >> >> ret = setsockopt(sock, SOL_PACKET, type, &ring->req, >>sizeof(ring->req)); >> if (ret == -1) { >> + __odp_errno = errno; >> ODP_ERR("setsockopt(pkt mmap): %s\n", strerror(errno)); >> return -1; >> } >> @@ -630,6 +640,7 @@ static int mmap_setup_ring(int sock, struct ring >>*ring, int type) >> ring->rd_len = ring->rd_num * sizeof(*ring->rd); >> ring->rd = malloc(ring->rd_len); >> if (ring->rd == NULL) { >> + __odp_errno = errno; >> ODP_ERR("malloc(): %s\n", strerror(errno)); >> return -1; >> } >> @@ -654,6 +665,7 @@ static int mmap_sock(pkt_sock_mmap_t *pkt_sock) >> MAP_SHARED | MAP_LOCKED | MAP_POPULATE, sock, 0); >> >> if (pkt_sock->mmap_base == MAP_FAILED) { >> + __odp_errno = errno; >> ODP_ERR("mmap rx&tx buffer failed: %s\n", >>strerror(errno)); >> return -1; >> } >> @@ -701,6 +713,7 @@ static int mmap_bind_sock(pkt_sock_mmap_t >>*pkt_sock, const char *netdev) >> ret = bind(pkt_sock->sockfd, (struct sockaddr *)&pkt_sock->ll, >> sizeof(pkt_sock->ll)); >> if (ret == -1) { >> + __odp_errno = errno; >> ODP_ERR("bind(to IF): %s\n", strerror(errno)); >> return -1; >> } >> @@ -719,6 +732,7 @@ static int mmap_store_hw_addr(pkt_sock_mmap_t >>*const pkt_sock, >> snprintf(ethreq.ifr_name, IFNAMSIZ, "%s", netdev); >> ret = ioctl(pkt_sock->sockfd, SIOCGIFHWADDR, ðreq); >> if (ret != 0) { >> + __odp_errno = errno; >> ODP_ERR("ioctl(SIOCGIFHWADDR): %s\n", strerror(errno)); >> return -1; >> } >> @@ -775,6 +789,7 @@ int setup_pkt_sock_mmap(pkt_sock_mmap_t *const >>pkt_sock, const char *netdev, >> >> if_idx = if_nametoindex(netdev); >> if (if_idx == 0) { >> + __odp_errno = errno; >> ODP_ERR("if_nametoindex(): %s\n", strerror(errno)); >> return -1; >> } >> @@ -796,6 +811,7 @@ int close_pkt_sock_mmap(pkt_sock_mmap_t *const >>pkt_sock) >> { >> mmap_unmap_sock(pkt_sock); >> if (pkt_sock->sockfd != -1 && close(pkt_sock->sockfd) != 0) { >> + __odp_errno = errno; >> ODP_ERR("close(sockfd): %s\n", strerror(errno)); >> return -1; >> } >> diff --git a/platform/linux-generic/odp_timer.c >>b/platform/linux-generic/odp_timer.c >> index 3ba32a1..a8e00f8 100644 >> --- a/platform/linux-generic/odp_timer.c >> +++ b/platform/linux-generic/odp_timer.c >> @@ -50,6 +50,7 @@ >> #include <odp_time.h> >> #include <odp_timer.h> >> #include <odp_timer_internal.h> >> +#include <odp_errno.h> >> >> #define TMO_UNUSED ((uint64_t)0xFFFFFFFFFFFFFFFF) >> /* TMO_INACTIVE is or-ed with the expiration tick to indicate an >>expired timer. >> @@ -210,7 +211,7 @@ static odp_timer_pool *odp_timer_pool_new( >> if (odp_unlikely(tp_idx >= MAX_TIMER_POOLS)) { >> /* Restore the previous value */ >> odp_atomic_sub_u32(&num_timer_pools, 1); >> - errno = ENFILE; /* Table overflow */ >> + __odp_errno = ENFILE; /* Table overflow */ >> return NULL; >> } >> size_t sz0 = ODP_ALIGN_ROUNDUP(sizeof(odp_timer_pool), >> @@ -295,7 +296,7 @@ static inline odp_timer_t >>timer_alloc(odp_timer_pool *tp, >> _ODP_MEMMODEL_RLS); >> hdl = tp_idx_to_handle(tp, idx); >> } else { >> - errno = ENFILE; /* Reusing file table overflow */ >> + __odp_errno = ENFILE; /* Reusing file table overflow */ >> hdl = ODP_TIMER_INVALID; >> } >> odp_spinlock_unlock(&tp->lock); >> -- >> 1.9.1 >> >> >> >> _______________________________________________ >> lng-odp mailing list >> [email protected] >> http://lists.linaro.org/mailman/listinfo/lng-odp > -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782 _______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
