| Message ID | 1411746134-4925-1-git-send-email-mike.holmes@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 09/26/2014 07:42 PM, Mike Holmes wrote: > @@ -311,10 +306,9 @@ static void link_bufs(pool_entry_t *pool) > hdr_size = sizeof(odp_timeout_hdr_t); > } else if (buf_type == ODP_BUFFER_TYPE_ANY) { > hdr_size = sizeof(odp_any_buffer_hdr_t); > - } else { > - ODP_ERR("odp_buffer_pool_create: Bad type %i\n", buf_type); > - exit(0); > - } missing "}". Please compile code before sending. Maxim. > + else > + ODP_ABORT("odp_buffer_pool_create: Bad type %i\n", buf_type); > + > >
Maybe the Linux kernel coding style isn't so smart after all... With braces on their own lines, vertically aligned, it is unlikely that this bug would have slipped through. And the code would have looked so much clearer, at least to me because my brain likes symmetry. -- Ola On 29 September 2014 17:45, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > On 09/26/2014 07:42 PM, Mike Holmes wrote: > >> @@ -311,10 +306,9 @@ static void link_bufs(pool_entry_t *pool) >> hdr_size = sizeof(odp_timeout_hdr_t); >> } else if (buf_type == ODP_BUFFER_TYPE_ANY) { >> hdr_size = sizeof(odp_any_buffer_hdr_t); >> - } else { >> - ODP_ERR("odp_buffer_pool_create: Bad type %i\n", >> buf_type); >> - exit(0); >> - } >> > > missing "}". Please compile code before sending. > > Maxim. > > + else >> + ODP_ABORT("odp_buffer_pool_create: Bad type %i\n", >> buf_type); >> + >> >> > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
Sorry, I thought I sent a don't look at this comment to this thread, Thank you Maxim I have a new version. As for coding style I really believe we should not discuss it at all! :) But enforce calling odp/scripts/odp_check on all source files, then a machine takes care of the style - consistent for all AND compatible with the linux kernel style. On 29 September 2014 12:31, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > Maybe the Linux kernel coding style isn't so smart after all... With > braces on their own lines, vertically aligned, it is unlikely that this bug > would have slipped through. And the code would have looked so much clearer, > at least to me because my brain likes symmetry. > > -- Ola > > > On 29 September 2014 17:45, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > >> >> On 09/26/2014 07:42 PM, Mike Holmes wrote: >> >>> @@ -311,10 +306,9 @@ static void link_bufs(pool_entry_t *pool) >>> hdr_size = sizeof(odp_timeout_hdr_t); >>> } else if (buf_type == ODP_BUFFER_TYPE_ANY) { >>> hdr_size = sizeof(odp_any_buffer_hdr_t); >>> - } else { >>> - ODP_ERR("odp_buffer_pool_create: Bad type %i\n", >>> buf_type); >>> - exit(0); >>> - } >>> >> >> missing "}". Please compile code before sending. >> >> Maxim. >> >> + else >>> + ODP_ABORT("odp_buffer_pool_create: Bad type %i\n", >>> buf_type); >>> + >>> >>> >> >> >> _______________________________________________ >> 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 > >
diff --git a/platform/linux-generic/include/api/odp_debug.h b/platform/linux-generic/include/api/odp_debug.h index e8f6003..344b0a9 100644 --- a/platform/linux-generic/include/api/odp_debug.h +++ b/platform/linux-generic/include/api/odp_debug.h @@ -13,6 +13,7 @@ #define ODP_DEBUG_H_ #include <stdio.h> +#include <stdlib.h> #ifdef __cplusplus extern "C" { @@ -76,8 +77,19 @@ extern "C" { * Print output to stderr (file, line and function). */ #define ODP_ERR(fmt, ...) \ - fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \ - __LINE__, __func__, ##__VA_ARGS__) +do { fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \ + __LINE__, __func__, ##__VA_ARGS__); \ +} while (0) + +/** + * Print output to stderr (file, line and function), + * then abort. + */ +#define ODP_ABORT(fmt, ...) \ +do { fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \ + __LINE__, __func__, ##__VA_ARGS__); \ + abort(); \ +} while (0) #ifdef __cplusplus } diff --git a/platform/linux-generic/odp_buffer_pool.c b/platform/linux-generic/odp_buffer_pool.c index f54a0c4..a981b51 100644 --- a/platform/linux-generic/odp_buffer_pool.c +++ b/platform/linux-generic/odp_buffer_pool.c @@ -98,10 +98,8 @@ static inline void set_handle(odp_buffer_hdr_t *hdr, odp_buffer_pool_t pool_hdl = pool->s.pool_hdl; uint32_t pool_id = pool_handle_to_index(pool_hdl); - if (pool_id >= ODP_CONFIG_BUFFER_POOLS) { - ODP_ERR("set_handle: Bad pool handle %u\n", pool_hdl); - exit(0); - } + if (pool_id >= ODP_CONFIG_BUFFER_POOLS) + ODP_ABORT("set_handle: Bad pool handle %u\n", pool_hdl); if (index > ODP_BUFFER_MAX_INDEX) ODP_ERR("set_handle: Bad buffer index\n"); @@ -218,15 +216,13 @@ static void add_chunk(pool_entry_t *pool, odp_buffer_chunk_hdr_t *chunk_hdr) static void check_align(pool_entry_t *pool, odp_buffer_hdr_t *hdr) { if (!ODP_ALIGNED_CHECK_POWER_2(hdr->addr, pool->s.user_align)) { - ODP_ERR("check_align: user data align error %p, align %zu\n", - hdr->addr, pool->s.user_align); - exit(0); + ODP_ABORT("check_align: user data align error %p, align %zu\n", + hdr->addr, pool->s.user_align); } if (!ODP_ALIGNED_CHECK_POWER_2(hdr, ODP_CACHE_LINE_SIZE)) { - ODP_ERR("check_align: hdr align error %p, align %i\n", - hdr, ODP_CACHE_LINE_SIZE); - exit(0); + ODP_ABORT("check_align: hdr align error %p, align %i\n", + hdr, ODP_CACHE_LINE_SIZE); } } @@ -264,8 +260,7 @@ static void fill_hdr(void *ptr, pool_entry_t *pool, uint32_t index, buf_data = any_hdr->buf_data; break; default: - ODP_ERR("Bad buffer type\n"); - exit(0); + ODP_ABORT("Bad buffer type\n"); } memset(hdr, 0, size); @@ -311,10 +306,9 @@ static void link_bufs(pool_entry_t *pool) hdr_size = sizeof(odp_timeout_hdr_t); } else if (buf_type == ODP_BUFFER_TYPE_ANY) { hdr_size = sizeof(odp_any_buffer_hdr_t); - } else { - ODP_ERR("odp_buffer_pool_create: Bad type %i\n", buf_type); - exit(0); - } + else + ODP_ABORT("odp_buffer_pool_create: Bad type %i\n", buf_type); + /* Chunk must fit into buffer data area.*/ min_size = sizeof(odp_buffer_chunk_hdr_t) - hdr_size; diff --git a/platform/linux-generic/odp_time.c b/platform/linux-generic/odp_time.c index 181294a..faece0e 100644 --- a/platform/linux-generic/odp_time.c +++ b/platform/linux-generic/odp_time.c @@ -59,8 +59,7 @@ uint64_t odp_time_get_cycles(void) ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); if (ret != 0) { - ODP_ERR("clock_gettime failed\n"); - exit(EXIT_FAILURE); + ODP_ABORT("clock_gettime failed\n"); } hz = odp_sys_cpu_hz();
Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- v2: Removed odp_check formatting platform/linux-generic/include/api/odp_debug.h | 16 ++++++++++++++-- platform/linux-generic/odp_buffer_pool.c | 26 ++++++++++---------------- platform/linux-generic/odp_time.c | 3 +-- 3 files changed, 25 insertions(+), 20 deletions(-)