| Message ID | 1411405849-31746-1-git-send-email-mike.holmes@linaro.org |
|---|---|
| State | Rejected |
| Headers | show |
Hi,
About formatting, are you sure the white space changes are acceptable by checkpatch. Maybe checkpatch does not warn you, because it does not see the whole picture. Just checking that we don't introduce new style errors. Currently there are only two errors.
perl scripts/checkpatch.pl -f platform/linux-generic/odp_buffer_pool.c
CHECK: Alignment should match open parenthesis
#50: FILE: linux-generic/odp_buffer_pool.c:50:
+ODP_STATIC_ASSERT((sizeof(union buffer_type_any_u) % 8) == 0,
+ "BUFFER_TYPE_ANY_U__SIZE_ERR");
ERROR: need consistent spacing around '-' (ctx:WxV)
#91: FILE: linux-generic/odp_buffer_pool.c:91:
+ return pool_hdl -1;
^
WARNING: Avoid CamelCase: <PRIxPTR>
#534: FILE: linux-generic/odp_buffer_pool.c:534:
+ printf(" buf base 0x%"PRIxPTR"\n", pool->s.buf_base);
total: 1 errors, 1 warnings, 1 checks, 577 lines checked
NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS
platform/linux-generic/odp_buffer_pool.c has style problems, pl
> -----Original Message-----
> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> bounces@lists.linaro.org] On Behalf Of ext Mike Holmes
> Sent: Monday, September 22, 2014 8:11 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [PATCH] Add ODP_ABORT to eliminate calls to exit
>
> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
> ---
>
> Also cleaned up checkpatch wanrings
>
> platform/linux-generic/include/api/odp_debug.h | 16 +++++++-
> platform/linux-generic/odp_buffer_pool.c | 51 ++++++++++++---------
> -----
> platform/linux-generic/odp_time.c | 3 +-
> 3 files changed, 38 insertions(+), 32 deletions(-)
>
> 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..68d7630 100644
> --- a/platform/linux-generic/odp_buffer_pool.c
> +++ b/platform/linux-generic/odp_buffer_pool.c
> @@ -47,7 +47,7 @@ union buffer_type_any_u {
> };
>
> ODP_STATIC_ASSERT((sizeof(union buffer_type_any_u) % 8) == 0,
> - "BUFFER_TYPE_ANY_U__SIZE_ERR");
> + "BUFFER_TYPE_ANY_U__SIZE_ERR");
>
> /* Any buffer type header */
> typedef struct {
> @@ -88,7 +88,7 @@ static inline odp_buffer_pool_t
> pool_index_to_handle(uint32_t pool_id)
>
> static inline uint32_t pool_handle_to_index(odp_buffer_pool_t pool_hdl)
> {
> - return pool_hdl -1;
> + return pool_hdl - 1;
> }
>
>
> @@ -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");
> @@ -146,7 +144,8 @@ static odp_buffer_hdr_t *index_to_hdr(pool_entry_t
> *pool, uint32_t index)
> {
> odp_buffer_hdr_t *hdr;
>
> - hdr = (odp_buffer_hdr_t *)(pool->s.buf_base + index * pool-
> >s.buf_size);
> + hdr = (odp_buffer_hdr_t *)(pool->s.buf_base + index
> + * pool->s.buf_size);
I think it's a 80 char line, and breaking that makes it less readable.
> return hdr;
> }
>
> @@ -172,7 +171,7 @@ static uint32_t rem_buf_index(odp_buffer_chunk_hdr_t
> *chunk_hdr)
>
>
> static odp_buffer_chunk_hdr_t *next_chunk(pool_entry_t *pool,
> - odp_buffer_chunk_hdr_t *chunk_hdr)
> + odp_buffer_chunk_hdr_t *chunk_hdr)
Parameter list should be aligned to opening parenthesis, right?
> {
> uint32_t index;
>
> @@ -218,15 +217,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 +261,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);
> @@ -303,18 +299,17 @@ static void link_bufs(pool_entry_t *pool)
> pool_size = pool->s.pool_size;
> pool_base = (uintptr_t) pool->s.pool_base_addr;
>
> - if (buf_type == ODP_BUFFER_TYPE_RAW) {
> + if (buf_type == ODP_BUFFER_TYPE_RAW)
> hdr_size = sizeof(odp_raw_buffer_hdr_t);
> - } else if (buf_type == ODP_BUFFER_TYPE_PACKET) {
> + else if (buf_type == ODP_BUFFER_TYPE_PACKET)
> hdr_size = sizeof(odp_packet_hdr_t);
> - } else if (buf_type == ODP_BUFFER_TYPE_TIMEOUT) {
> + else if (buf_type == ODP_BUFFER_TYPE_TIMEOUT)
> hdr_size = sizeof(odp_timeout_hdr_t);
> - } else if (buf_type == ODP_BUFFER_TYPE_ANY) {
> + 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;
> @@ -366,7 +361,7 @@ static void link_bufs(pool_entry_t *pool)
> add_chunk(pool, chunk_hdr);
>
> chunk_hdr = (odp_buffer_chunk_hdr_t *)index_to_hdr(pool,
> - index);
> + index);
Parameter list should be aligned to opening parenthesis, right?
> pool->s.num_bufs += ODP_BUFS_PER_CHUNK;
> pool_size -= ODP_BUFS_PER_CHUNK * tot_size;
> }
> @@ -374,9 +369,9 @@ static void link_bufs(pool_entry_t *pool)
>
>
> odp_buffer_pool_t odp_buffer_pool_create(const char *name,
> - void *base_addr, uint64_t size,
> - size_t buf_size, size_t buf_align,
> - int buf_type)
> + void *base_addr, uint64_t size,
> + size_t buf_size, size_t buf_align,
> + int buf_type)
Parameter list should be aligned to opening parenthesis, right?
-Petri
> {
> odp_buffer_pool_t pool_hdl = ODP_BUFFER_POOL_INVALID;
> pool_entry_t *pool;
> 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();
> --
> 1.9.1
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
Thanks Petri,
I used scripts/odp_check and checkpatch to clear all warnings so I have
something different to you - my repos and the scent patch pass, and I re
downloaded the patch form the list and it also passed, but let me look into
it some more.
------------------------------------------------------------------------------
mike@fedora1:~/git/odp$ ./scripts/checkpatch.pl
0001-Update-ODP_ERR-to-call-abort.patch
total: 0 errors, 0 warnings, 0 checks, 60 lines checked
NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS
0001-Update-ODP_ERR-to-call-abort.patch has no obvious style problems and
is ready for submission.
------------------------------------------------------------------------------
mike@fedora1:~/git/odp$ ./scripts/checkpatch.pl
0001-Update-ODP_ERR-to-call-abort.patch
total: 0 errors, 0 warnings, 0 checks, 60 lines checked
NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS
0001-Update-ODP_ERR-to-call-abort.patch has no obvious style problems and
is ready for submission.
mike@fedora1:~/git/odp$ perl scripts/checkpatch.pl -f
platform/linux-generic/odp_buffer_pool.c
WARNING: Avoid CamelCase: <PRIxPTR>
#529: FILE: linux-generic/odp_buffer_pool.c:529:
+ printf(" buf base 0x%"PRIxPTR"\n", pool->s.buf_base);
total: 0 errors, 1 warnings, 0 checks, 572 lines checked
NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS
platform/linux-generic/odp_buffer_pool.c has style problems, please review.
If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
------------------------------------------------------------------------------
Down from the list again.
mike@fedora1:~/git/odp$ ./scripts/checkpatch.pl
PATCH_Add_ODP_ABORT_to_eliminate_calls_to_exit.mbox
total: 0 errors, 0 warnings, 0 checks, 155 lines checked
NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS
PATCH_Add_ODP_ABORT_to_eliminate_calls_to_exit.mbox has no obvious style
problems and is ready for submission.
On 23 September 2014 03:35, Savolainen, Petri (NSN - FI/Espoo) <
petri.savolainen@nsn.com> wrote:
> Hi,
>
> About formatting, are you sure the white space changes are acceptable by
> checkpatch. Maybe checkpatch does not warn you, because it does not see the
> whole picture. Just checking that we don't introduce new style errors.
> Currently there are only two errors.
>
> perl scripts/checkpatch.pl -f platform/linux-generic/odp_buffer_pool.c
>
> CHECK: Alignment should match open parenthesis
> #50: FILE: linux-generic/odp_buffer_pool.c:50:
> +ODP_STATIC_ASSERT((sizeof(union buffer_type_any_u) % 8) == 0,
> + "BUFFER_TYPE_ANY_U__SIZE_ERR");
>
> ERROR: need consistent spacing around '-' (ctx:WxV)
> #91: FILE: linux-generic/odp_buffer_pool.c:91:
> + return pool_hdl -1;
> ^
>
> WARNING: Avoid CamelCase: <PRIxPTR>
> #534: FILE: linux-generic/odp_buffer_pool.c:534:
> + printf(" buf base 0x%"PRIxPTR"\n", pool->s.buf_base);
>
> total: 1 errors, 1 warnings, 1 checks, 577 lines checked
>
> NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS
>
> platform/linux-generic/odp_buffer_pool.c has style problems, pl
>
>
> > -----Original Message-----
> > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> > bounces@lists.linaro.org] On Behalf Of ext Mike Holmes
> > Sent: Monday, September 22, 2014 8:11 PM
> > To: lng-odp@lists.linaro.org
> > Subject: [lng-odp] [PATCH] Add ODP_ABORT to eliminate calls to exit
> >
> > Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
> > ---
> >
> > Also cleaned up checkpatch wanrings
> >
> > platform/linux-generic/include/api/odp_debug.h | 16 +++++++-
> > platform/linux-generic/odp_buffer_pool.c | 51
> ++++++++++++---------
> > -----
> > platform/linux-generic/odp_time.c | 3 +-
> > 3 files changed, 38 insertions(+), 32 deletions(-)
> >
> > 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..68d7630 100644
> > --- a/platform/linux-generic/odp_buffer_pool.c
> > +++ b/platform/linux-generic/odp_buffer_pool.c
> > @@ -47,7 +47,7 @@ union buffer_type_any_u {
> > };
> >
> > ODP_STATIC_ASSERT((sizeof(union buffer_type_any_u) % 8) == 0,
> > - "BUFFER_TYPE_ANY_U__SIZE_ERR");
> > + "BUFFER_TYPE_ANY_U__SIZE_ERR");
> >
> > /* Any buffer type header */
> > typedef struct {
> > @@ -88,7 +88,7 @@ static inline odp_buffer_pool_t
> > pool_index_to_handle(uint32_t pool_id)
> >
> > static inline uint32_t pool_handle_to_index(odp_buffer_pool_t pool_hdl)
> > {
> > - return pool_hdl -1;
> > + return pool_hdl - 1;
> > }
> >
> >
> > @@ -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");
> > @@ -146,7 +144,8 @@ static odp_buffer_hdr_t *index_to_hdr(pool_entry_t
> > *pool, uint32_t index)
> > {
> > odp_buffer_hdr_t *hdr;
> >
> > - hdr = (odp_buffer_hdr_t *)(pool->s.buf_base + index * pool-
> > >s.buf_size);
> > + hdr = (odp_buffer_hdr_t *)(pool->s.buf_base + index
> > + * pool->s.buf_size);
>
> I think it's a 80 char line, and breaking that makes it less readable.
>
> > return hdr;
> > }
> >
> > @@ -172,7 +171,7 @@ static uint32_t rem_buf_index(odp_buffer_chunk_hdr_t
> > *chunk_hdr)
> >
> >
> > static odp_buffer_chunk_hdr_t *next_chunk(pool_entry_t *pool,
> > - odp_buffer_chunk_hdr_t
> *chunk_hdr)
> > + odp_buffer_chunk_hdr_t *chunk_hdr)
>
> Parameter list should be aligned to opening parenthesis, right?
>
> > {
> > uint32_t index;
> >
> > @@ -218,15 +217,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 +261,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);
> > @@ -303,18 +299,17 @@ static void link_bufs(pool_entry_t *pool)
> > pool_size = pool->s.pool_size;
> > pool_base = (uintptr_t) pool->s.pool_base_addr;
> >
> > - if (buf_type == ODP_BUFFER_TYPE_RAW) {
> > + if (buf_type == ODP_BUFFER_TYPE_RAW)
> > hdr_size = sizeof(odp_raw_buffer_hdr_t);
> > - } else if (buf_type == ODP_BUFFER_TYPE_PACKET) {
> > + else if (buf_type == ODP_BUFFER_TYPE_PACKET)
> > hdr_size = sizeof(odp_packet_hdr_t);
> > - } else if (buf_type == ODP_BUFFER_TYPE_TIMEOUT) {
> > + else if (buf_type == ODP_BUFFER_TYPE_TIMEOUT)
> > hdr_size = sizeof(odp_timeout_hdr_t);
> > - } else if (buf_type == ODP_BUFFER_TYPE_ANY) {
> > + 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;
> > @@ -366,7 +361,7 @@ static void link_bufs(pool_entry_t *pool)
> > add_chunk(pool, chunk_hdr);
> >
> > chunk_hdr = (odp_buffer_chunk_hdr_t *)index_to_hdr(pool,
> > - index);
> > + index);
>
> Parameter list should be aligned to opening parenthesis, right?
>
>
> > pool->s.num_bufs += ODP_BUFS_PER_CHUNK;
> > pool_size -= ODP_BUFS_PER_CHUNK * tot_size;
> > }
> > @@ -374,9 +369,9 @@ static void link_bufs(pool_entry_t *pool)
> >
> >
> > odp_buffer_pool_t odp_buffer_pool_create(const char *name,
> > - void *base_addr, uint64_t size,
> > - size_t buf_size, size_t buf_align,
> > - int buf_type)
> > + void *base_addr, uint64_t size,
> > + size_t buf_size, size_t buf_align,
> > + int buf_type)
>
> Parameter list should be aligned to opening parenthesis, right?
>
>
> -Petri
>
>
> > {
> > odp_buffer_pool_t pool_hdl = ODP_BUFFER_POOL_INVALID;
> > pool_entry_t *pool;
> > 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();
> > --
> > 1.9.1
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
Did you run checkpatch for the whole file (-f option) after modifications? I’m speculating that check for the patch may pass (due to incomplete view), but check for the file fail. -Petri From: ext Mike Holmes [mailto:mike.holmes@linaro.org] Sent: Tuesday, September 23, 2014 2:39 PM To: Savolainen, Petri (NSN - FI/Espoo) Cc: lng-odp@lists.linaro.org Subject: Re: [lng-odp] [PATCH] Add ODP_ABORT to eliminate calls to exit Thanks Petri, I used scripts/odp_check and checkpatch to clear all warnings so I have something different to you - my repos and the scent patch pass, and I re downloaded the patch form the list and it also passed, but let me look into it some more. ------------------------------------------------------------------------------ mike@fedora1:~/git/odp$ ./scripts/checkpatch.pl<http://checkpatch.pl> 0001-Update-ODP_ERR-to-call-abort.patch total: 0 errors, 0 warnings, 0 checks, 60 lines checked NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS 0001-Update-ODP_ERR-to-call-abort.patch has no obvious style problems and is ready for submission. ------------------------------------------------------------------------------ mike@fedora1:~/git/odp$ ./scripts/checkpatch.pl<http://checkpatch.pl> 0001-Update-ODP_ERR-to-call-abort.patch total: 0 errors, 0 warnings, 0 checks, 60 lines checked NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS 0001-Update-ODP_ERR-to-call-abort.patch has no obvious style problems and is ready for submission. mike@fedora1:~/git/odp$ perl scripts/checkpatch.pl<http://checkpatch.pl> -f platform/linux-generic/odp_buffer_pool.c WARNING: Avoid CamelCase: <PRIxPTR> #529: FILE: linux-generic/odp_buffer_pool.c:529: + printf(" buf base 0x%"PRIxPTR"\n", pool->s.buf_base); total: 0 errors, 1 warnings, 0 checks, 572 lines checked NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS platform/linux-generic/odp_buffer_pool.c has style problems, please review. If any of these errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. ------------------------------------------------------------------------------ Down from the list again. mike@fedora1:~/git/odp$ ./scripts/checkpatch.pl<http://checkpatch.pl> PATCH_Add_ODP_ABORT_to_eliminate_calls_to_exit.mbox total: 0 errors, 0 warnings, 0 checks, 155 lines checked NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS PATCH_Add_ODP_ABORT_to_eliminate_calls_to_exit.mbox has no obvious style problems and is ready for submission. On 23 September 2014 03:35, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com<mailto:petri.savolainen@nsn.com>> wrote: Hi, About formatting, are you sure the white space changes are acceptable by checkpatch. Maybe checkpatch does not warn you, because it does not see the whole picture. Just checking that we don't introduce new style errors. Currently there are only two errors. perl scripts/checkpatch.pl<http://checkpatch.pl> -f platform/linux-generic/odp_buffer_pool.c CHECK: Alignment should match open parenthesis #50: FILE: linux-generic/odp_buffer_pool.c:50: +ODP_STATIC_ASSERT((sizeof(union buffer_type_any_u) % 8) == 0, + "BUFFER_TYPE_ANY_U__SIZE_ERR"); ERROR: need consistent spacing around '-' (ctx:WxV) #91: FILE: linux-generic/odp_buffer_pool.c:91: + return pool_hdl -1; ^ WARNING: Avoid CamelCase: <PRIxPTR> #534: FILE: linux-generic/odp_buffer_pool.c:534: + printf(" buf base 0x%"PRIxPTR"\n", pool->s.buf_base); total: 1 errors, 1 warnings, 1 checks, 577 lines checked NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS platform/linux-generic/odp_buffer_pool.c has style problems, pl > -----Original Message----- > From: lng-odp-bounces@lists.linaro.org<mailto:lng-odp-bounces@lists.linaro.org> [mailto:lng-odp-<mailto:lng-odp-> > bounces@lists.linaro.org<mailto:bounces@lists.linaro.org>] On Behalf Of ext Mike Holmes > Sent: Monday, September 22, 2014 8:11 PM > To: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > Subject: [lng-odp] [PATCH] Add ODP_ABORT to eliminate calls to exit > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>> > --- > > Also cleaned up checkpatch wanrings > > platform/linux-generic/include/api/odp_debug.h | 16 +++++++- > platform/linux-generic/odp_buffer_pool.c | 51 ++++++++++++--------- > ----- > platform/linux-generic/odp_time.c | 3 +- > 3 files changed, 38 insertions(+), 32 deletions(-) > > 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..68d7630 100644 > --- a/platform/linux-generic/odp_buffer_pool.c > +++ b/platform/linux-generic/odp_buffer_pool.c > @@ -47,7 +47,7 @@ union buffer_type_any_u { > }; > > ODP_STATIC_ASSERT((sizeof(union buffer_type_any_u) % 8) == 0, > - "BUFFER_TYPE_ANY_U__SIZE_ERR"); > + "BUFFER_TYPE_ANY_U__SIZE_ERR"); > > /* Any buffer type header */ > typedef struct { > @@ -88,7 +88,7 @@ static inline odp_buffer_pool_t > pool_index_to_handle(uint32_t pool_id) > > static inline uint32_t pool_handle_to_index(odp_buffer_pool_t pool_hdl) > { > - return pool_hdl -1; > + return pool_hdl - 1; > } > > > @@ -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"); > @@ -146,7 +144,8 @@ static odp_buffer_hdr_t *index_to_hdr(pool_entry_t > *pool, uint32_t index) > { > odp_buffer_hdr_t *hdr; > > - hdr = (odp_buffer_hdr_t *)(pool->s.buf_base + index * pool- > >s.buf_size); > + hdr = (odp_buffer_hdr_t *)(pool->s.buf_base + index > + * pool->s.buf_size); I think it's a 80 char line, and breaking that makes it less readable. > return hdr; > } > > @@ -172,7 +171,7 @@ static uint32_t rem_buf_index(odp_buffer_chunk_hdr_t > *chunk_hdr) > > > static odp_buffer_chunk_hdr_t *next_chunk(pool_entry_t *pool, > - odp_buffer_chunk_hdr_t *chunk_hdr) > + odp_buffer_chunk_hdr_t *chunk_hdr) Parameter list should be aligned to opening parenthesis, right? > { > uint32_t index; > > @@ -218,15 +217,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 +261,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); > @@ -303,18 +299,17 @@ static void link_bufs(pool_entry_t *pool) > pool_size = pool->s.pool_size; > pool_base = (uintptr_t) pool->s.pool_base_addr; > > - if (buf_type == ODP_BUFFER_TYPE_RAW) { > + if (buf_type == ODP_BUFFER_TYPE_RAW) > hdr_size = sizeof(odp_raw_buffer_hdr_t); > - } else if (buf_type == ODP_BUFFER_TYPE_PACKET) { > + else if (buf_type == ODP_BUFFER_TYPE_PACKET) > hdr_size = sizeof(odp_packet_hdr_t); > - } else if (buf_type == ODP_BUFFER_TYPE_TIMEOUT) { > + else if (buf_type == ODP_BUFFER_TYPE_TIMEOUT) > hdr_size = sizeof(odp_timeout_hdr_t); > - } else if (buf_type == ODP_BUFFER_TYPE_ANY) { > + 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; > @@ -366,7 +361,7 @@ static void link_bufs(pool_entry_t *pool) > add_chunk(pool, chunk_hdr); > > chunk_hdr = (odp_buffer_chunk_hdr_t *)index_to_hdr(pool, > - index); > + index); Parameter list should be aligned to opening parenthesis, right? > pool->s.num_bufs += ODP_BUFS_PER_CHUNK; > pool_size -= ODP_BUFS_PER_CHUNK * tot_size; > } > @@ -374,9 +369,9 @@ static void link_bufs(pool_entry_t *pool) > > > odp_buffer_pool_t odp_buffer_pool_create(const char *name, > - void *base_addr, uint64_t size, > - size_t buf_size, size_t buf_align, > - int buf_type) > + void *base_addr, uint64_t size, > + size_t buf_size, size_t buf_align, > + int buf_type) Parameter list should be aligned to opening parenthesis, right? -Petri > { > odp_buffer_pool_t pool_hdl = ODP_BUFFER_POOL_INVALID; > pool_entry_t *pool; > 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(); > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp -- Mike Holmes Linaro Technical Manager / Lead LNG - ODP
Curious odp_check does use -f on the source file - and clean file fixes the issues, but the final patch itself does show extra whitespace errors on top of that when run once more with -f. I had assumed the first pass was enough, thanks for pointing that out. On 23 September 2014 07:48, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> wrote: > Did you run checkpatch for the whole file (-f option) after > modifications? I’m speculating that check for the patch may pass (due to > incomplete view), but check for the file fail. > > > > -Petri > > > > > > *From:* ext Mike Holmes [mailto:mike.holmes@linaro.org] > *Sent:* Tuesday, September 23, 2014 2:39 PM > *To:* Savolainen, Petri (NSN - FI/Espoo) > *Cc:* lng-odp@lists.linaro.org > *Subject:* Re: [lng-odp] [PATCH] Add ODP_ABORT to eliminate calls to exit > > > > Thanks Petri, > > > > I used scripts/odp_check and checkpatch to clear all warnings so I have > something different to you - my repos and the scent patch pass, and I re > downloaded the patch form the list and it also passed, but let me look into > it some more. > > > > > ------------------------------------------------------------------------------ > > mike@fedora1:~/git/odp$ ./scripts/checkpatch.pl > 0001-Update-ODP_ERR-to-call-abort.patch > > total: 0 errors, 0 warnings, 0 checks, 60 lines checked > > > > NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS > > > > 0001-Update-ODP_ERR-to-call-abort.patch has no obvious style problems and > is ready for submission. > > > > > ------------------------------------------------------------------------------ > > mike@fedora1:~/git/odp$ ./scripts/checkpatch.pl > 0001-Update-ODP_ERR-to-call-abort.patch > > total: 0 errors, 0 warnings, 0 checks, 60 lines checked > > > > NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS > > > > 0001-Update-ODP_ERR-to-call-abort.patch has no obvious style problems and > is ready for submission. > > mike@fedora1:~/git/odp$ perl scripts/checkpatch.pl -f > platform/linux-generic/odp_buffer_pool.c > > WARNING: Avoid CamelCase: <PRIxPTR> > > #529: FILE: linux-generic/odp_buffer_pool.c:529: > > + printf(" buf base 0x%"PRIxPTR"\n", pool->s.buf_base); > > > > total: 0 errors, 1 warnings, 0 checks, 572 lines checked > > > > NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS > > > > platform/linux-generic/odp_buffer_pool.c has style problems, please review. > > > > If any of these errors are false positives, please report > > them to the maintainer, see CHECKPATCH in MAINTAINERS. > > > > > ------------------------------------------------------------------------------ > > > > Down from the list again. > > > > mike@fedora1:~/git/odp$ ./scripts/checkpatch.pl > PATCH_Add_ODP_ABORT_to_eliminate_calls_to_exit.mbox > > total: 0 errors, 0 warnings, 0 checks, 155 lines checked > > > > NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS > > > > PATCH_Add_ODP_ABORT_to_eliminate_calls_to_exit.mbox has no obvious style > problems and is ready for submission. > > > > > > > > On 23 September 2014 03:35, Savolainen, Petri (NSN - FI/Espoo) < > petri.savolainen@nsn.com> wrote: > > Hi, > > About formatting, are you sure the white space changes are acceptable by > checkpatch. Maybe checkpatch does not warn you, because it does not see the > whole picture. Just checking that we don't introduce new style errors. > Currently there are only two errors. > > perl scripts/checkpatch.pl -f platform/linux-generic/odp_buffer_pool.c > > CHECK: Alignment should match open parenthesis > #50: FILE: linux-generic/odp_buffer_pool.c:50: > +ODP_STATIC_ASSERT((sizeof(union buffer_type_any_u) % 8) == 0, > + "BUFFER_TYPE_ANY_U__SIZE_ERR"); > > ERROR: need consistent spacing around '-' (ctx:WxV) > #91: FILE: linux-generic/odp_buffer_pool.c:91: > + return pool_hdl -1; > ^ > > WARNING: Avoid CamelCase: <PRIxPTR> > #534: FILE: linux-generic/odp_buffer_pool.c:534: > + printf(" buf base 0x%"PRIxPTR"\n", pool->s.buf_base); > > total: 1 errors, 1 warnings, 1 checks, 577 lines checked > > NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS > > platform/linux-generic/odp_buffer_pool.c has style problems, pl > > > > > -----Original Message----- > > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > > bounces@lists.linaro.org] On Behalf Of ext Mike Holmes > > Sent: Monday, September 22, 2014 8:11 PM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [PATCH] Add ODP_ABORT to eliminate calls to exit > > > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > > --- > > > > Also cleaned up checkpatch wanrings > > > > platform/linux-generic/include/api/odp_debug.h | 16 +++++++- > > platform/linux-generic/odp_buffer_pool.c | 51 > ++++++++++++--------- > > ----- > > platform/linux-generic/odp_time.c | 3 +- > > 3 files changed, 38 insertions(+), 32 deletions(-) > > > > 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..68d7630 100644 > > --- a/platform/linux-generic/odp_buffer_pool.c > > +++ b/platform/linux-generic/odp_buffer_pool.c > > @@ -47,7 +47,7 @@ union buffer_type_any_u { > > }; > > > > ODP_STATIC_ASSERT((sizeof(union buffer_type_any_u) % 8) == 0, > > - "BUFFER_TYPE_ANY_U__SIZE_ERR"); > > + "BUFFER_TYPE_ANY_U__SIZE_ERR"); > > > > /* Any buffer type header */ > > typedef struct { > > @@ -88,7 +88,7 @@ static inline odp_buffer_pool_t > > pool_index_to_handle(uint32_t pool_id) > > > > static inline uint32_t pool_handle_to_index(odp_buffer_pool_t pool_hdl) > > { > > - return pool_hdl -1; > > + return pool_hdl - 1; > > } > > > > > > @@ -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"); > > @@ -146,7 +144,8 @@ static odp_buffer_hdr_t *index_to_hdr(pool_entry_t > > *pool, uint32_t index) > > { > > odp_buffer_hdr_t *hdr; > > > > - hdr = (odp_buffer_hdr_t *)(pool->s.buf_base + index * pool- > > >s.buf_size); > > + hdr = (odp_buffer_hdr_t *)(pool->s.buf_base + index > > + * pool->s.buf_size); > > I think it's a 80 char line, and breaking that makes it less readable. > > > return hdr; > > } > > > > @@ -172,7 +171,7 @@ static uint32_t rem_buf_index(odp_buffer_chunk_hdr_t > > *chunk_hdr) > > > > > > static odp_buffer_chunk_hdr_t *next_chunk(pool_entry_t *pool, > > - odp_buffer_chunk_hdr_t > *chunk_hdr) > > + odp_buffer_chunk_hdr_t *chunk_hdr) > > Parameter list should be aligned to opening parenthesis, right? > > > > { > > uint32_t index; > > > > @@ -218,15 +217,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 +261,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); > > @@ -303,18 +299,17 @@ static void link_bufs(pool_entry_t *pool) > > pool_size = pool->s.pool_size; > > pool_base = (uintptr_t) pool->s.pool_base_addr; > > > > - if (buf_type == ODP_BUFFER_TYPE_RAW) { > > + if (buf_type == ODP_BUFFER_TYPE_RAW) > > hdr_size = sizeof(odp_raw_buffer_hdr_t); > > - } else if (buf_type == ODP_BUFFER_TYPE_PACKET) { > > + else if (buf_type == ODP_BUFFER_TYPE_PACKET) > > hdr_size = sizeof(odp_packet_hdr_t); > > - } else if (buf_type == ODP_BUFFER_TYPE_TIMEOUT) { > > + else if (buf_type == ODP_BUFFER_TYPE_TIMEOUT) > > hdr_size = sizeof(odp_timeout_hdr_t); > > - } else if (buf_type == ODP_BUFFER_TYPE_ANY) { > > + 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; > > @@ -366,7 +361,7 @@ static void link_bufs(pool_entry_t *pool) > > add_chunk(pool, chunk_hdr); > > > > chunk_hdr = (odp_buffer_chunk_hdr_t *)index_to_hdr(pool, > > - index); > > + index); > > Parameter list should be aligned to opening parenthesis, right? > > > > pool->s.num_bufs += ODP_BUFS_PER_CHUNK; > > pool_size -= ODP_BUFS_PER_CHUNK * tot_size; > > } > > @@ -374,9 +369,9 @@ static void link_bufs(pool_entry_t *pool) > > > > > > odp_buffer_pool_t odp_buffer_pool_create(const char *name, > > - void *base_addr, uint64_t size, > > - size_t buf_size, size_t buf_align, > > - int buf_type) > > + void *base_addr, uint64_t size, > > + size_t buf_size, size_t buf_align, > > + int buf_type) > > Parameter list should be aligned to opening parenthesis, right? > > > -Petri > > > > { > > odp_buffer_pool_t pool_hdl = ODP_BUFFER_POOL_INVALID; > > pool_entry_t *pool; > > 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(); > > -- > > 1.9.1 > > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > -- > > *Mike Holmes* > > Linaro Technical Manager / Lead > > 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..68d7630 100644 --- a/platform/linux-generic/odp_buffer_pool.c +++ b/platform/linux-generic/odp_buffer_pool.c @@ -47,7 +47,7 @@ union buffer_type_any_u { }; ODP_STATIC_ASSERT((sizeof(union buffer_type_any_u) % 8) == 0, - "BUFFER_TYPE_ANY_U__SIZE_ERR"); + "BUFFER_TYPE_ANY_U__SIZE_ERR"); /* Any buffer type header */ typedef struct { @@ -88,7 +88,7 @@ static inline odp_buffer_pool_t pool_index_to_handle(uint32_t pool_id) static inline uint32_t pool_handle_to_index(odp_buffer_pool_t pool_hdl) { - return pool_hdl -1; + return pool_hdl - 1; } @@ -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"); @@ -146,7 +144,8 @@ static odp_buffer_hdr_t *index_to_hdr(pool_entry_t *pool, uint32_t index) { odp_buffer_hdr_t *hdr; - hdr = (odp_buffer_hdr_t *)(pool->s.buf_base + index * pool->s.buf_size); + hdr = (odp_buffer_hdr_t *)(pool->s.buf_base + index + * pool->s.buf_size); return hdr; } @@ -172,7 +171,7 @@ static uint32_t rem_buf_index(odp_buffer_chunk_hdr_t *chunk_hdr) static odp_buffer_chunk_hdr_t *next_chunk(pool_entry_t *pool, - odp_buffer_chunk_hdr_t *chunk_hdr) + odp_buffer_chunk_hdr_t *chunk_hdr) { uint32_t index; @@ -218,15 +217,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 +261,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); @@ -303,18 +299,17 @@ static void link_bufs(pool_entry_t *pool) pool_size = pool->s.pool_size; pool_base = (uintptr_t) pool->s.pool_base_addr; - if (buf_type == ODP_BUFFER_TYPE_RAW) { + if (buf_type == ODP_BUFFER_TYPE_RAW) hdr_size = sizeof(odp_raw_buffer_hdr_t); - } else if (buf_type == ODP_BUFFER_TYPE_PACKET) { + else if (buf_type == ODP_BUFFER_TYPE_PACKET) hdr_size = sizeof(odp_packet_hdr_t); - } else if (buf_type == ODP_BUFFER_TYPE_TIMEOUT) { + else if (buf_type == ODP_BUFFER_TYPE_TIMEOUT) hdr_size = sizeof(odp_timeout_hdr_t); - } else if (buf_type == ODP_BUFFER_TYPE_ANY) { + 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; @@ -366,7 +361,7 @@ static void link_bufs(pool_entry_t *pool) add_chunk(pool, chunk_hdr); chunk_hdr = (odp_buffer_chunk_hdr_t *)index_to_hdr(pool, - index); + index); pool->s.num_bufs += ODP_BUFS_PER_CHUNK; pool_size -= ODP_BUFS_PER_CHUNK * tot_size; } @@ -374,9 +369,9 @@ static void link_bufs(pool_entry_t *pool) odp_buffer_pool_t odp_buffer_pool_create(const char *name, - void *base_addr, uint64_t size, - size_t buf_size, size_t buf_align, - int buf_type) + void *base_addr, uint64_t size, + size_t buf_size, size_t buf_align, + int buf_type) { odp_buffer_pool_t pool_hdl = ODP_BUFFER_POOL_INVALID; pool_entry_t *pool; 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> --- Also cleaned up checkpatch wanrings platform/linux-generic/include/api/odp_debug.h | 16 +++++++- platform/linux-generic/odp_buffer_pool.c | 51 ++++++++++++-------------- platform/linux-generic/odp_time.c | 3 +- 3 files changed, 38 insertions(+), 32 deletions(-)