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) < [email protected]> 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:[email protected]] > *Sent:* Tuesday, September 23, 2014 2:39 PM > *To:* Savolainen, Petri (NSN - FI/Espoo) > *Cc:* [email protected] > *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) < > [email protected]> 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: [email protected] [mailto:lng-odp- > > [email protected]] On Behalf Of ext Mike Holmes > > Sent: Monday, September 22, 2014 8:11 PM > > To: [email protected] > > Subject: [lng-odp] [PATCH] Add ODP_ABORT to eliminate calls to exit > > > > Signed-off-by: Mike Holmes <[email protected]> > > --- > > > > 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 > > [email protected] > > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > -- > > *Mike Holmes* > > Linaro Technical Manager / Lead > > LNG - ODP > -- *Mike Holmes* Linaro Technical Manager / Lead LNG - ODP
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
