linux-generic: Replace printf with ODP_PRI

Message ID	1416335416-7740-1-git-send-email-mike.holmes@linaro.org
State	Rejected
Headers	show Return-Path: <patchwork-forward+bncBCT4HJVWSENBBY5AV2RQKGQEZQSU27Q@linaro.org> Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.182 as permitted sender) client-ip=209.85.217.182; Received-SPF: none (google.com: lng-odp-bounces@lists.linaro.org does not designate permitted sender hosts) client-ip=54.225.227.206; From: Mike Holmes <mike.holmes@linaro.org> To: lng-odp@lists.linaro.org Date: Tue, 18 Nov 2014 13:30:16 -0500 Message-Id: <1416335416-7740-1-git-send-email-mike.holmes@linaro.org> Subject: [lng-odp] [PATCH] linux-generic: Replace printf with ODP_PRI Precedence: list MIME-Version: 1.0 Errors-To: lng-odp-bounces@lists.linaro.org Sender: lng-odp-bounces@lists.linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit

Mike Holmes Nov. 18, 2014, 6:30 p.m. UTC

Current ODP APIs that print internal data on demand for the application do so
via  printf.

Introduce ODP_PRI so that APIs that produce output may be channeled though
ODP_LOG to match the other logging types.

Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
---

This patch does not impact the behaviour of any current example or test.

A follow on should implement the application replaceable ODP_LOG function
via weak linking.

 platform/linux-generic/include/api/odp_debug.h | 14 +++++++-
 platform/linux-generic/odp_buffer.c            |  4 +--
 platform/linux-generic/odp_buffer_pool.c       | 44 +++++++++++++-------------
 platform/linux-generic/odp_packet.c            |  2 +-
 platform/linux-generic/odp_shared_memory.c     | 30 +++++++++---------
 5 files changed, 53 insertions(+), 41 deletions(-)

Bill Fischofer Nov. 18, 2014, 6:50 p.m. UTC | #1

PRI is neither a standard nor obvious abbreviation for PRINT.  Why not just
call it ODP_PRINT()?  Is saving two characters worth the confusion?

Bill

On Tue, Nov 18, 2014 at 12:30 PM, Mike Holmes <mike.holmes@linaro.org>
wrote:

> Current ODP APIs that print internal data on demand for the application do
> so
> via  printf.
>
> Introduce ODP_PRI so that APIs that produce output may be channeled though
> ODP_LOG to match the other logging types.
>
> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
> ---
>
> This patch does not impact the behaviour of any current example or test.
>
> A follow on should implement the application replaceable ODP_LOG function
> via weak linking.
>
>  platform/linux-generic/include/api/odp_debug.h | 14 +++++++-
>  platform/linux-generic/odp_buffer.c            |  4 +--
>  platform/linux-generic/odp_buffer_pool.c       | 44
> +++++++++++++-------------
>  platform/linux-generic/odp_packet.c            |  2 +-
>  platform/linux-generic/odp_shared_memory.c     | 30 +++++++++---------
>  5 files changed, 53 insertions(+), 41 deletions(-)
>
> diff --git a/platform/linux-generic/include/api/odp_debug.h
> b/platform/linux-generic/include/api/odp_debug.h
> index c9b2edd..1a6fbda 100644
> --- a/platform/linux-generic/include/api/odp_debug.h
> +++ b/platform/linux-generic/include/api/odp_debug.h
> @@ -76,7 +76,8 @@ typedef enum odp_log_level {
>         ODP_LOG_DBG,
>         ODP_LOG_ERR,
>         ODP_LOG_UNIMPLEMENTED,
> -       ODP_LOG_ABORT
> +       ODP_LOG_ABORT,
> +       ODP_LOG_PRI,
>  } odp_log_level_e;
>
>  /**
> @@ -94,6 +95,10 @@ do { \
>                         fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>                         __LINE__, __func__, ##__VA_ARGS__); \
>                 break; \
> +       case ODP_LOG_PRI: \
> +               fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
> +               __LINE__, __func__, ##__VA_ARGS__); \
> +               break; \
>         case ODP_LOG_ABORT: \
>                 fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \
>                 __LINE__, __func__, ##__VA_ARGS__); \
> @@ -111,6 +116,13 @@ do { \
>  } while (0)
>
>  /**
> + * Printing macro, which prints output when the application
> + * calls one of the ODP APIs specifically for dumping internal data.
> + */
> +#define ODP_PRI(fmt, ...) \
> +               ODP_LOG(ODP_LOG_PRI, fmt, ##__VA_ARGS__)
> +
> +/**
>   * Debug printing macro, which prints output when DEBUG flag is set.
>   */
>  #define ODP_DBG(fmt, ...) \
> diff --git a/platform/linux-generic/odp_buffer.c
> b/platform/linux-generic/odp_buffer.c
> index e54e0e7..2ee0461 100644
> --- a/platform/linux-generic/odp_buffer.c
> +++ b/platform/linux-generic/odp_buffer.c
> @@ -52,7 +52,7 @@ int odp_buffer_snprint(char *str, size_t n, odp_buffer_t
> buf)
>         int len = 0;
>
>         if (!odp_buffer_is_valid(buf)) {
> -               printf("Buffer is not valid.\n");
> +               ODP_PRI("Buffer is not valid.\n");
>                 return len;
>         }
>
> @@ -98,7 +98,7 @@ void odp_buffer_print(odp_buffer_t buf)
>         len = odp_buffer_snprint(str, max_len-1, buf);
>         str[len] = 0;
>
> -       printf("\n%s\n", str);
> +       ODP_PRI("\n%s\n", str);
>  }
>
>  void odp_buffer_copy_scatter(odp_buffer_t buf_dst, odp_buffer_t buf_src)
> diff --git a/platform/linux-generic/odp_buffer_pool.c
> b/platform/linux-generic/odp_buffer_pool.c
> index a48d7d6..3555166 100644
> --- a/platform/linux-generic/odp_buffer_pool.c
> +++ b/platform/linux-generic/odp_buffer_pool.c
> @@ -523,20 +523,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
> pool_hdl)
>         pool_id = pool_handle_to_index(pool_hdl);
>         pool    = get_pool_entry(pool_id);
>
> -       printf("Pool info\n");
> -       printf("---------\n");
> -       printf("  pool          %i\n",           pool->s.pool_hdl);
> -       printf("  name          %s\n",           pool->s.name);
> -       printf("  pool base     %p\n",           pool->s.pool_base_addr);
> -       printf("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
> -       printf("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
> -       printf("  buf size      %zu\n",          pool->s.user_size);
> -       printf("  buf align     %zu\n",          pool->s.user_align);
> -       printf("  hdr size      %zu\n",          pool->s.hdr_size);
> -       printf("  alloc size    %zu\n",          pool->s.buf_size);
> -       printf("  offset to hdr %zu\n",          pool->s.buf_offset);
> -       printf("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
> -       printf("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
> +       ODP_PRI("Pool info\n");
> +       ODP_PRI("---------\n");
> +       ODP_PRI("  pool          %i\n",           pool->s.pool_hdl);
> +       ODP_PRI("  name          %s\n",           pool->s.name);
> +       ODP_PRI("  pool base     %p\n",           pool->s.pool_base_addr);
> +       ODP_PRI("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
> +       ODP_PRI("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
> +       ODP_PRI("  buf size      %zu\n",          pool->s.user_size);
> +       ODP_PRI("  buf align     %zu\n",          pool->s.user_align);
> +       ODP_PRI("  hdr size      %zu\n",          pool->s.hdr_size);
> +       ODP_PRI("  alloc size    %zu\n",          pool->s.buf_size);
> +       ODP_PRI("  offset to hdr %zu\n",          pool->s.buf_offset);
> +       ODP_PRI("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
> +       ODP_PRI("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>
>         /* first chunk */
>         chunk_hdr = pool->s.head;
> @@ -546,7 +546,7 @@ void odp_buffer_pool_print(odp_buffer_pool_t pool_hdl)
>                 return;
>         }
>
> -       printf("\n  First chunk\n");
> +       ODP_PRI("\n  First chunk\n");
>
>         for (i = 0; i < chunk_hdr->chunk.num_bufs - 1; i++) {
>                 uint32_t index;
> @@ -555,20 +555,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
> pool_hdl)
>                 index = chunk_hdr->chunk.buf_index[i];
>                 hdr   = index_to_hdr(pool, index);
>
> -               printf("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr,
> index);
> +               ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr,
> index);
>         }
>
> -       printf("  [%i] addr %p, id %"PRIu32"\n", i,
> chunk_hdr->buf_hdr.addr,
> -              chunk_hdr->buf_hdr.index);
> +       ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i,
> chunk_hdr->buf_hdr.addr,
> +               chunk_hdr->buf_hdr.index);
>
>         /* next chunk */
>         chunk_hdr = next_chunk(pool, chunk_hdr);
>
>         if (chunk_hdr) {
> -               printf("  Next chunk\n");
> -               printf("  addr %p, id %"PRIu32"\n",
> chunk_hdr->buf_hdr.addr,
> -                      chunk_hdr->buf_hdr.index);
> +               ODP_PRI("  Next chunk\n");
> +               ODP_PRI("  addr %p, id %"PRIu32"\n",
> chunk_hdr->buf_hdr.addr,
> +                       chunk_hdr->buf_hdr.index);
>         }
>
> -       printf("\n");
> +       ODP_PRI("\n");
>  }
> diff --git a/platform/linux-generic/odp_packet.c
> b/platform/linux-generic/odp_packet.c
> index 82ea879..7fe9ed0 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -346,7 +346,7 @@ void odp_packet_print(odp_packet_t pkt)
>                         "  input        %u\n", hdr->input);
>         str[len] = '\0';
>
> -       printf("\n%s\n", str);
> +       ODP_PRI("\n%s\n", str);
>  }
>
>  int odp_packet_copy(odp_packet_t pkt_dst, odp_packet_t pkt_src)
> diff --git a/platform/linux-generic/odp_shared_memory.c
> b/platform/linux-generic/odp_shared_memory.c
> index 24a5d60..cd4415d 100644
> --- a/platform/linux-generic/odp_shared_memory.c
> +++ b/platform/linux-generic/odp_shared_memory.c
> @@ -285,14 +285,14 @@ void odp_shm_print_all(void)
>  {
>         int i;
>
> -       printf("\nShared memory\n");
> -       printf("--------------\n");
> -       printf("  page size:      %"PRIu64" kB\n", odp_sys_page_size() /
> 1024);
> -       printf("  huge page size: %"PRIu64" kB\n",
> -              odp_sys_huge_page_size() / 1024);
> -       printf("\n");
> +       ODP_PRI("\nShared memory\n");
> +       ODP_PRI("--------------\n");
> +       ODP_PRI("  page size:      %"PRIu64" kB\n", odp_sys_page_size() /
> 1024);
> +       ODP_PRI("  huge page size: %"PRIu64" kB\n",
> +               odp_sys_huge_page_size() / 1024);
> +       ODP_PRI("\n");
>
> -       printf("  id name                       kB align huge addr\n");
> +       ODP_PRI("  id name                       kB align huge addr\n");
>
>         for (i = 0; i < ODP_SHM_NUM_BLOCKS; i++) {
>                 odp_shm_block_t *block;
> @@ -300,15 +300,15 @@ void odp_shm_print_all(void)
>                 block = &odp_shm_tbl->block[i];
>
>                 if (block->addr) {
> -                       printf("  %2i %-24s %4"PRIu64"  %4"PRIu64" %2c
>  %p\n",
> -                              i,
> -                              block->name,
> -                              block->size/1024,
> -                              block->align,
> -                              (block->huge ? '*' : ' '),
> -                              block->addr);
> +                       ODP_PRI("  %2i %-24s %4"PRIu64"  %4"PRIu64" %2c
>  %p\n",
> +                               i,
> +                               block->name,
> +                               block->size/1024,
> +                               block->align,
> +                               (block->huge ? '*' : ' '),
> +                               block->addr);
>                 }
>         }
>
> -       printf("\n");
> +       ODP_PRI("\n");
>  }
> --
> 2.1.0
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>

Mike Holmes Nov. 18, 2014, 6:56 p.m. UTC | #2

On 18 November 2014 13:50, Bill Fischofer <bill.fischofer@linaro.org> wrote:

> PRI is neither a standard nor obvious abbreviation for PRINT.  Why not
> just call it ODP_PRINT()?  Is saving two characters worth the confusion?
>

I thought it was, here is my rational.

Because the length difference generates all sorts of manual formatting
issues if you swap it between ODP_PRINT ODP_DBG, for example some lines pop
over 80 chars, the parentheses no longer line up etc
I experienced this when creating in the patches of the examples and the
test to remove their use of of ODP_DBG.

Also 80 chars is so few and with print strings being harder to break neatly
over lines I felt the truncation helped there also.

But I am not wedded to ODP_PRI, I can change it if there is consensus.


>
> Bill
>
> On Tue, Nov 18, 2014 at 12:30 PM, Mike Holmes <mike.holmes@linaro.org>
> wrote:
>
>> Current ODP APIs that print internal data on demand for the application
>> do so
>> via  printf.
>>
>> Introduce ODP_PRI so that APIs that produce output may be channeled though
>> ODP_LOG to match the other logging types.
>>
>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
>> ---
>>
>> This patch does not impact the behaviour of any current example or test.
>>
>> A follow on should implement the application replaceable ODP_LOG function
>> via weak linking.
>>
>>  platform/linux-generic/include/api/odp_debug.h | 14 +++++++-
>>  platform/linux-generic/odp_buffer.c            |  4 +--
>>  platform/linux-generic/odp_buffer_pool.c       | 44
>> +++++++++++++-------------
>>  platform/linux-generic/odp_packet.c            |  2 +-
>>  platform/linux-generic/odp_shared_memory.c     | 30 +++++++++---------
>>  5 files changed, 53 insertions(+), 41 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/api/odp_debug.h
>> b/platform/linux-generic/include/api/odp_debug.h
>> index c9b2edd..1a6fbda 100644
>> --- a/platform/linux-generic/include/api/odp_debug.h
>> +++ b/platform/linux-generic/include/api/odp_debug.h
>> @@ -76,7 +76,8 @@ typedef enum odp_log_level {
>>         ODP_LOG_DBG,
>>         ODP_LOG_ERR,
>>         ODP_LOG_UNIMPLEMENTED,
>> -       ODP_LOG_ABORT
>> +       ODP_LOG_ABORT,
>> +       ODP_LOG_PRI,
>>  } odp_log_level_e;
>>
>>  /**
>> @@ -94,6 +95,10 @@ do { \
>>                         fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>>                         __LINE__, __func__, ##__VA_ARGS__); \
>>                 break; \
>> +       case ODP_LOG_PRI: \
>> +               fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>> +               __LINE__, __func__, ##__VA_ARGS__); \
>> +               break; \
>>         case ODP_LOG_ABORT: \
>>                 fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \
>>                 __LINE__, __func__, ##__VA_ARGS__); \
>> @@ -111,6 +116,13 @@ do { \
>>  } while (0)
>>
>>  /**
>> + * Printing macro, which prints output when the application
>> + * calls one of the ODP APIs specifically for dumping internal data.
>> + */
>> +#define ODP_PRI(fmt, ...) \
>> +               ODP_LOG(ODP_LOG_PRI, fmt, ##__VA_ARGS__)
>> +
>> +/**
>>   * Debug printing macro, which prints output when DEBUG flag is set.
>>   */
>>  #define ODP_DBG(fmt, ...) \
>> diff --git a/platform/linux-generic/odp_buffer.c
>> b/platform/linux-generic/odp_buffer.c
>> index e54e0e7..2ee0461 100644
>> --- a/platform/linux-generic/odp_buffer.c
>> +++ b/platform/linux-generic/odp_buffer.c
>> @@ -52,7 +52,7 @@ int odp_buffer_snprint(char *str, size_t n,
>> odp_buffer_t buf)
>>         int len = 0;
>>
>>         if (!odp_buffer_is_valid(buf)) {
>> -               printf("Buffer is not valid.\n");
>> +               ODP_PRI("Buffer is not valid.\n");
>>                 return len;
>>         }
>>
>> @@ -98,7 +98,7 @@ void odp_buffer_print(odp_buffer_t buf)
>>         len = odp_buffer_snprint(str, max_len-1, buf);
>>         str[len] = 0;
>>
>> -       printf("\n%s\n", str);
>> +       ODP_PRI("\n%s\n", str);
>>  }
>>
>>  void odp_buffer_copy_scatter(odp_buffer_t buf_dst, odp_buffer_t buf_src)
>> diff --git a/platform/linux-generic/odp_buffer_pool.c
>> b/platform/linux-generic/odp_buffer_pool.c
>> index a48d7d6..3555166 100644
>> --- a/platform/linux-generic/odp_buffer_pool.c
>> +++ b/platform/linux-generic/odp_buffer_pool.c
>> @@ -523,20 +523,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>> pool_hdl)
>>         pool_id = pool_handle_to_index(pool_hdl);
>>         pool    = get_pool_entry(pool_id);
>>
>> -       printf("Pool info\n");
>> -       printf("---------\n");
>> -       printf("  pool          %i\n",           pool->s.pool_hdl);
>> -       printf("  name          %s\n",           pool->s.name);
>> -       printf("  pool base     %p\n",           pool->s.pool_base_addr);
>> -       printf("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
>> -       printf("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
>> -       printf("  buf size      %zu\n",          pool->s.user_size);
>> -       printf("  buf align     %zu\n",          pool->s.user_align);
>> -       printf("  hdr size      %zu\n",          pool->s.hdr_size);
>> -       printf("  alloc size    %zu\n",          pool->s.buf_size);
>> -       printf("  offset to hdr %zu\n",          pool->s.buf_offset);
>> -       printf("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
>> -       printf("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>> +       ODP_PRI("Pool info\n");
>> +       ODP_PRI("---------\n");
>> +       ODP_PRI("  pool          %i\n",           pool->s.pool_hdl);
>> +       ODP_PRI("  name          %s\n",           pool->s.name);
>> +       ODP_PRI("  pool base     %p\n",           pool->s.pool_base_addr);
>> +       ODP_PRI("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
>> +       ODP_PRI("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
>> +       ODP_PRI("  buf size      %zu\n",          pool->s.user_size);
>> +       ODP_PRI("  buf align     %zu\n",          pool->s.user_align);
>> +       ODP_PRI("  hdr size      %zu\n",          pool->s.hdr_size);
>> +       ODP_PRI("  alloc size    %zu\n",          pool->s.buf_size);
>> +       ODP_PRI("  offset to hdr %zu\n",          pool->s.buf_offset);
>> +       ODP_PRI("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
>> +       ODP_PRI("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>>
>>         /* first chunk */
>>         chunk_hdr = pool->s.head;
>> @@ -546,7 +546,7 @@ void odp_buffer_pool_print(odp_buffer_pool_t pool_hdl)
>>                 return;
>>         }
>>
>> -       printf("\n  First chunk\n");
>> +       ODP_PRI("\n  First chunk\n");
>>
>>         for (i = 0; i < chunk_hdr->chunk.num_bufs - 1; i++) {
>>                 uint32_t index;
>> @@ -555,20 +555,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>> pool_hdl)
>>                 index = chunk_hdr->chunk.buf_index[i];
>>                 hdr   = index_to_hdr(pool, index);
>>
>> -               printf("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr,
>> index);
>> +               ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr,
>> index);
>>         }
>>
>> -       printf("  [%i] addr %p, id %"PRIu32"\n", i,
>> chunk_hdr->buf_hdr.addr,
>> -              chunk_hdr->buf_hdr.index);
>> +       ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i,
>> chunk_hdr->buf_hdr.addr,
>> +               chunk_hdr->buf_hdr.index);
>>
>>         /* next chunk */
>>         chunk_hdr = next_chunk(pool, chunk_hdr);
>>
>>         if (chunk_hdr) {
>> -               printf("  Next chunk\n");
>> -               printf("  addr %p, id %"PRIu32"\n",
>> chunk_hdr->buf_hdr.addr,
>> -                      chunk_hdr->buf_hdr.index);
>> +               ODP_PRI("  Next chunk\n");
>> +               ODP_PRI("  addr %p, id %"PRIu32"\n",
>> chunk_hdr->buf_hdr.addr,
>> +                       chunk_hdr->buf_hdr.index);
>>         }
>>
>> -       printf("\n");
>> +       ODP_PRI("\n");
>>  }
>> diff --git a/platform/linux-generic/odp_packet.c
>> b/platform/linux-generic/odp_packet.c
>> index 82ea879..7fe9ed0 100644
>> --- a/platform/linux-generic/odp_packet.c
>> +++ b/platform/linux-generic/odp_packet.c
>> @@ -346,7 +346,7 @@ void odp_packet_print(odp_packet_t pkt)
>>                         "  input        %u\n", hdr->input);
>>         str[len] = '\0';
>>
>> -       printf("\n%s\n", str);
>> +       ODP_PRI("\n%s\n", str);
>>  }
>>
>>  int odp_packet_copy(odp_packet_t pkt_dst, odp_packet_t pkt_src)
>> diff --git a/platform/linux-generic/odp_shared_memory.c
>> b/platform/linux-generic/odp_shared_memory.c
>> index 24a5d60..cd4415d 100644
>> --- a/platform/linux-generic/odp_shared_memory.c
>> +++ b/platform/linux-generic/odp_shared_memory.c
>> @@ -285,14 +285,14 @@ void odp_shm_print_all(void)
>>  {
>>         int i;
>>
>> -       printf("\nShared memory\n");
>> -       printf("--------------\n");
>> -       printf("  page size:      %"PRIu64" kB\n", odp_sys_page_size() /
>> 1024);
>> -       printf("  huge page size: %"PRIu64" kB\n",
>> -              odp_sys_huge_page_size() / 1024);
>> -       printf("\n");
>> +       ODP_PRI("\nShared memory\n");
>> +       ODP_PRI("--------------\n");
>> +       ODP_PRI("  page size:      %"PRIu64" kB\n", odp_sys_page_size() /
>> 1024);
>> +       ODP_PRI("  huge page size: %"PRIu64" kB\n",
>> +               odp_sys_huge_page_size() / 1024);
>> +       ODP_PRI("\n");
>>
>> -       printf("  id name                       kB align huge addr\n");
>> +       ODP_PRI("  id name                       kB align huge addr\n");
>>
>>         for (i = 0; i < ODP_SHM_NUM_BLOCKS; i++) {
>>                 odp_shm_block_t *block;
>> @@ -300,15 +300,15 @@ void odp_shm_print_all(void)
>>                 block = &odp_shm_tbl->block[i];
>>
>>                 if (block->addr) {
>> -                       printf("  %2i %-24s %4"PRIu64"  %4"PRIu64" %2c
>>  %p\n",
>> -                              i,
>> -                              block->name,
>> -                              block->size/1024,
>> -                              block->align,
>> -                              (block->huge ? '*' : ' '),
>> -                              block->addr);
>> +                       ODP_PRI("  %2i %-24s %4"PRIu64"  %4"PRIu64" %2c
>>  %p\n",
>> +                               i,
>> +                               block->name,
>> +                               block->size/1024,
>> +                               block->align,
>> +                               (block->huge ? '*' : ' '),
>> +                               block->addr);
>>                 }
>>         }
>>
>> -       printf("\n");
>> +       ODP_PRI("\n");
>>  }
>> --
>> 2.1.0
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>

Bill Fischofer Nov. 18, 2014, 7:04 p.m. UTC | #3

The standard abbreviation for PRINT would be PRT, but that's easily typo'd
with PTR, which is the universal standard abbreviation for pointer.

Why not just ODP_LOG() then?

Torturing the syntax to comply with a procrustean rule on line length
points to a bigger issue with the 'standard' we allowed ourself to get
saddled with, but that's beyond the scope of this patch.  :)



On Tue, Nov 18, 2014 at 12:56 PM, Mike Holmes <mike.holmes@linaro.org>
wrote:

>
>
> On 18 November 2014 13:50, Bill Fischofer <bill.fischofer@linaro.org>
> wrote:
>
>> PRI is neither a standard nor obvious abbreviation for PRINT.  Why not
>> just call it ODP_PRINT()?  Is saving two characters worth the confusion?
>>
>
> I thought it was, here is my rational.
>
> Because the length difference generates all sorts of manual formatting
> issues if you swap it between ODP_PRINT ODP_DBG, for example some lines pop
> over 80 chars, the parentheses no longer line up etc
> I experienced this when creating in the patches of the examples and the
> test to remove their use of of ODP_DBG.
>
> Also 80 chars is so few and with print strings being harder to break
> neatly over lines I felt the truncation helped there also.
>
> But I am not wedded to ODP_PRI, I can change it if there is consensus.
>
>
>>
>> Bill
>>
>> On Tue, Nov 18, 2014 at 12:30 PM, Mike Holmes <mike.holmes@linaro.org>
>> wrote:
>>
>>> Current ODP APIs that print internal data on demand for the application
>>> do so
>>> via  printf.
>>>
>>> Introduce ODP_PRI so that APIs that produce output may be channeled
>>> though
>>> ODP_LOG to match the other logging types.
>>>
>>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
>>> ---
>>>
>>> This patch does not impact the behaviour of any current example or test.
>>>
>>> A follow on should implement the application replaceable ODP_LOG function
>>> via weak linking.
>>>
>>>  platform/linux-generic/include/api/odp_debug.h | 14 +++++++-
>>>  platform/linux-generic/odp_buffer.c            |  4 +--
>>>  platform/linux-generic/odp_buffer_pool.c       | 44
>>> +++++++++++++-------------
>>>  platform/linux-generic/odp_packet.c            |  2 +-
>>>  platform/linux-generic/odp_shared_memory.c     | 30 +++++++++---------
>>>  5 files changed, 53 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/platform/linux-generic/include/api/odp_debug.h
>>> b/platform/linux-generic/include/api/odp_debug.h
>>> index c9b2edd..1a6fbda 100644
>>> --- a/platform/linux-generic/include/api/odp_debug.h
>>> +++ b/platform/linux-generic/include/api/odp_debug.h
>>> @@ -76,7 +76,8 @@ typedef enum odp_log_level {
>>>         ODP_LOG_DBG,
>>>         ODP_LOG_ERR,
>>>         ODP_LOG_UNIMPLEMENTED,
>>> -       ODP_LOG_ABORT
>>> +       ODP_LOG_ABORT,
>>> +       ODP_LOG_PRI,
>>>  } odp_log_level_e;
>>>
>>>  /**
>>> @@ -94,6 +95,10 @@ do { \
>>>                         fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>>>                         __LINE__, __func__, ##__VA_ARGS__); \
>>>                 break; \
>>> +       case ODP_LOG_PRI: \
>>> +               fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>>> +               __LINE__, __func__, ##__VA_ARGS__); \
>>> +               break; \
>>>         case ODP_LOG_ABORT: \
>>>                 fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \
>>>                 __LINE__, __func__, ##__VA_ARGS__); \
>>> @@ -111,6 +116,13 @@ do { \
>>>  } while (0)
>>>
>>>  /**
>>> + * Printing macro, which prints output when the application
>>> + * calls one of the ODP APIs specifically for dumping internal data.
>>> + */
>>> +#define ODP_PRI(fmt, ...) \
>>> +               ODP_LOG(ODP_LOG_PRI, fmt, ##__VA_ARGS__)
>>> +
>>> +/**
>>>   * Debug printing macro, which prints output when DEBUG flag is set.
>>>   */
>>>  #define ODP_DBG(fmt, ...) \
>>> diff --git a/platform/linux-generic/odp_buffer.c
>>> b/platform/linux-generic/odp_buffer.c
>>> index e54e0e7..2ee0461 100644
>>> --- a/platform/linux-generic/odp_buffer.c
>>> +++ b/platform/linux-generic/odp_buffer.c
>>> @@ -52,7 +52,7 @@ int odp_buffer_snprint(char *str, size_t n,
>>> odp_buffer_t buf)
>>>         int len = 0;
>>>
>>>         if (!odp_buffer_is_valid(buf)) {
>>> -               printf("Buffer is not valid.\n");
>>> +               ODP_PRI("Buffer is not valid.\n");
>>>                 return len;
>>>         }
>>>
>>> @@ -98,7 +98,7 @@ void odp_buffer_print(odp_buffer_t buf)
>>>         len = odp_buffer_snprint(str, max_len-1, buf);
>>>         str[len] = 0;
>>>
>>> -       printf("\n%s\n", str);
>>> +       ODP_PRI("\n%s\n", str);
>>>  }
>>>
>>>  void odp_buffer_copy_scatter(odp_buffer_t buf_dst, odp_buffer_t buf_src)
>>> diff --git a/platform/linux-generic/odp_buffer_pool.c
>>> b/platform/linux-generic/odp_buffer_pool.c
>>> index a48d7d6..3555166 100644
>>> --- a/platform/linux-generic/odp_buffer_pool.c
>>> +++ b/platform/linux-generic/odp_buffer_pool.c
>>> @@ -523,20 +523,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>>> pool_hdl)
>>>         pool_id = pool_handle_to_index(pool_hdl);
>>>         pool    = get_pool_entry(pool_id);
>>>
>>> -       printf("Pool info\n");
>>> -       printf("---------\n");
>>> -       printf("  pool          %i\n",           pool->s.pool_hdl);
>>> -       printf("  name          %s\n",           pool->s.name);
>>> -       printf("  pool base     %p\n",           pool->s.pool_base_addr);
>>> -       printf("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
>>> -       printf("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
>>> -       printf("  buf size      %zu\n",          pool->s.user_size);
>>> -       printf("  buf align     %zu\n",          pool->s.user_align);
>>> -       printf("  hdr size      %zu\n",          pool->s.hdr_size);
>>> -       printf("  alloc size    %zu\n",          pool->s.buf_size);
>>> -       printf("  offset to hdr %zu\n",          pool->s.buf_offset);
>>> -       printf("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
>>> -       printf("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>>> +       ODP_PRI("Pool info\n");
>>> +       ODP_PRI("---------\n");
>>> +       ODP_PRI("  pool          %i\n",           pool->s.pool_hdl);
>>> +       ODP_PRI("  name          %s\n",           pool->s.name);
>>> +       ODP_PRI("  pool base     %p\n",
>>>  pool->s.pool_base_addr);
>>> +       ODP_PRI("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
>>> +       ODP_PRI("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
>>> +       ODP_PRI("  buf size      %zu\n",          pool->s.user_size);
>>> +       ODP_PRI("  buf align     %zu\n",          pool->s.user_align);
>>> +       ODP_PRI("  hdr size      %zu\n",          pool->s.hdr_size);
>>> +       ODP_PRI("  alloc size    %zu\n",          pool->s.buf_size);
>>> +       ODP_PRI("  offset to hdr %zu\n",          pool->s.buf_offset);
>>> +       ODP_PRI("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
>>> +       ODP_PRI("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>>>
>>>         /* first chunk */
>>>         chunk_hdr = pool->s.head;
>>> @@ -546,7 +546,7 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>>> pool_hdl)
>>>                 return;
>>>         }
>>>
>>> -       printf("\n  First chunk\n");
>>> +       ODP_PRI("\n  First chunk\n");
>>>
>>>         for (i = 0; i < chunk_hdr->chunk.num_bufs - 1; i++) {
>>>                 uint32_t index;
>>> @@ -555,20 +555,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>>> pool_hdl)
>>>                 index = chunk_hdr->chunk.buf_index[i];
>>>                 hdr   = index_to_hdr(pool, index);
>>>
>>> -               printf("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr,
>>> index);
>>> +               ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr,
>>> index);
>>>         }
>>>
>>> -       printf("  [%i] addr %p, id %"PRIu32"\n", i,
>>> chunk_hdr->buf_hdr.addr,
>>> -              chunk_hdr->buf_hdr.index);
>>> +       ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i,
>>> chunk_hdr->buf_hdr.addr,
>>> +               chunk_hdr->buf_hdr.index);
>>>
>>>         /* next chunk */
>>>         chunk_hdr = next_chunk(pool, chunk_hdr);
>>>
>>>         if (chunk_hdr) {
>>> -               printf("  Next chunk\n");
>>> -               printf("  addr %p, id %"PRIu32"\n",
>>> chunk_hdr->buf_hdr.addr,
>>> -                      chunk_hdr->buf_hdr.index);
>>> +               ODP_PRI("  Next chunk\n");
>>> +               ODP_PRI("  addr %p, id %"PRIu32"\n",
>>> chunk_hdr->buf_hdr.addr,
>>> +                       chunk_hdr->buf_hdr.index);
>>>         }
>>>
>>> -       printf("\n");
>>> +       ODP_PRI("\n");
>>>  }
>>> diff --git a/platform/linux-generic/odp_packet.c
>>> b/platform/linux-generic/odp_packet.c
>>> index 82ea879..7fe9ed0 100644
>>> --- a/platform/linux-generic/odp_packet.c
>>> +++ b/platform/linux-generic/odp_packet.c
>>> @@ -346,7 +346,7 @@ void odp_packet_print(odp_packet_t pkt)
>>>                         "  input        %u\n", hdr->input);
>>>         str[len] = '\0';
>>>
>>> -       printf("\n%s\n", str);
>>> +       ODP_PRI("\n%s\n", str);
>>>  }
>>>
>>>  int odp_packet_copy(odp_packet_t pkt_dst, odp_packet_t pkt_src)
>>> diff --git a/platform/linux-generic/odp_shared_memory.c
>>> b/platform/linux-generic/odp_shared_memory.c
>>> index 24a5d60..cd4415d 100644
>>> --- a/platform/linux-generic/odp_shared_memory.c
>>> +++ b/platform/linux-generic/odp_shared_memory.c
>>> @@ -285,14 +285,14 @@ void odp_shm_print_all(void)
>>>  {
>>>         int i;
>>>
>>> -       printf("\nShared memory\n");
>>> -       printf("--------------\n");
>>> -       printf("  page size:      %"PRIu64" kB\n", odp_sys_page_size() /
>>> 1024);
>>> -       printf("  huge page size: %"PRIu64" kB\n",
>>> -              odp_sys_huge_page_size() / 1024);
>>> -       printf("\n");
>>> +       ODP_PRI("\nShared memory\n");
>>> +       ODP_PRI("--------------\n");
>>> +       ODP_PRI("  page size:      %"PRIu64" kB\n", odp_sys_page_size()
>>> / 1024);
>>> +       ODP_PRI("  huge page size: %"PRIu64" kB\n",
>>> +               odp_sys_huge_page_size() / 1024);
>>> +       ODP_PRI("\n");
>>>
>>> -       printf("  id name                       kB align huge addr\n");
>>> +       ODP_PRI("  id name                       kB align huge addr\n");
>>>
>>>         for (i = 0; i < ODP_SHM_NUM_BLOCKS; i++) {
>>>                 odp_shm_block_t *block;
>>> @@ -300,15 +300,15 @@ void odp_shm_print_all(void)
>>>                 block = &odp_shm_tbl->block[i];
>>>
>>>                 if (block->addr) {
>>> -                       printf("  %2i %-24s %4"PRIu64"  %4"PRIu64" %2c
>>>  %p\n",
>>> -                              i,
>>> -                              block->name,
>>> -                              block->size/1024,
>>> -                              block->align,
>>> -                              (block->huge ? '*' : ' '),
>>> -                              block->addr);
>>> +                       ODP_PRI("  %2i %-24s %4"PRIu64"  %4"PRIu64" %2c
>>>  %p\n",
>>> +                               i,
>>> +                               block->name,
>>> +                               block->size/1024,
>>> +                               block->align,
>>> +                               (block->huge ? '*' : ' '),
>>> +                               block->addr);
>>>                 }
>>>         }
>>>
>>> -       printf("\n");
>>> +       ODP_PRI("\n");
>>>  }
>>> --
>>> 2.1.0
>>>
>>>
>>> _______________________________________________
>>> lng-odp mailing list
>>> lng-odp@lists.linaro.org
>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>>
>>
>>
>
>
> --
> *Mike Holmes*
> Linaro  Sr Technical Manager
> LNG - ODP
>

Mike Holmes Nov. 18, 2014, 7:22 p.m. UTC | #4

On 18 November 2014 14:04, Bill Fischofer <bill.fischofer@linaro.org> wrote:

> The standard abbreviation for PRINT would be PRT, but that's easily typo'd
> with PTR, which is the universal standard abbreviation for pointer.
>
> Why not just ODP_LOG() then?
>

That is already used, that is what all the functions funnel into, so to use
it we have to do some renaming of ODP_LOG to free it up for use in place of
ODP_PRI
ODP_LOG is the root and is what should be replaceable by the application as
it stands, but that patch has not gone in yet.


> Torturing the syntax to comply with a procrustean rule on line length
> points to a bigger issue with the 'standard' we allowed ourself to get
> saddled with, but that's beyond the scope of this patch.  :)
>

:)


>
>
>
> On Tue, Nov 18, 2014 at 12:56 PM, Mike Holmes <mike.holmes@linaro.org>
> wrote:
>
>>
>>
>> On 18 November 2014 13:50, Bill Fischofer <bill.fischofer@linaro.org>
>> wrote:
>>
>>> PRI is neither a standard nor obvious abbreviation for PRINT.  Why not
>>> just call it ODP_PRINT()?  Is saving two characters worth the confusion?
>>>
>>
>> I thought it was, here is my rational.
>>
>> Because the length difference generates all sorts of manual formatting
>> issues if you swap it between ODP_PRINT ODP_DBG, for example some lines pop
>> over 80 chars, the parentheses no longer line up etc
>> I experienced this when creating in the patches of the examples and the
>> test to remove their use of of ODP_DBG.
>>
>> Also 80 chars is so few and with print strings being harder to break
>> neatly over lines I felt the truncation helped there also.
>>
>> But I am not wedded to ODP_PRI, I can change it if there is consensus.
>>
>>
>>>
>>> Bill
>>>
>>> On Tue, Nov 18, 2014 at 12:30 PM, Mike Holmes <mike.holmes@linaro.org>
>>> wrote:
>>>
>>>> Current ODP APIs that print internal data on demand for the application
>>>> do so
>>>> via  printf.
>>>>
>>>> Introduce ODP_PRI so that APIs that produce output may be channeled
>>>> though
>>>> ODP_LOG to match the other logging types.
>>>>
>>>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
>>>> ---
>>>>
>>>> This patch does not impact the behaviour of any current example or test.
>>>>
>>>> A follow on should implement the application replaceable ODP_LOG
>>>> function
>>>> via weak linking.
>>>>
>>>>  platform/linux-generic/include/api/odp_debug.h | 14 +++++++-
>>>>  platform/linux-generic/odp_buffer.c            |  4 +--
>>>>  platform/linux-generic/odp_buffer_pool.c       | 44
>>>> +++++++++++++-------------
>>>>  platform/linux-generic/odp_packet.c            |  2 +-
>>>>  platform/linux-generic/odp_shared_memory.c     | 30 +++++++++---------
>>>>  5 files changed, 53 insertions(+), 41 deletions(-)
>>>>
>>>> diff --git a/platform/linux-generic/include/api/odp_debug.h
>>>> b/platform/linux-generic/include/api/odp_debug.h
>>>> index c9b2edd..1a6fbda 100644
>>>> --- a/platform/linux-generic/include/api/odp_debug.h
>>>> +++ b/platform/linux-generic/include/api/odp_debug.h
>>>> @@ -76,7 +76,8 @@ typedef enum odp_log_level {
>>>>         ODP_LOG_DBG,
>>>>         ODP_LOG_ERR,
>>>>         ODP_LOG_UNIMPLEMENTED,
>>>> -       ODP_LOG_ABORT
>>>> +       ODP_LOG_ABORT,
>>>> +       ODP_LOG_PRI,
>>>>  } odp_log_level_e;
>>>>
>>>>  /**
>>>> @@ -94,6 +95,10 @@ do { \
>>>>                         fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>>>>                         __LINE__, __func__, ##__VA_ARGS__); \
>>>>                 break; \
>>>> +       case ODP_LOG_PRI: \
>>>> +               fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>>>> +               __LINE__, __func__, ##__VA_ARGS__); \
>>>> +               break; \
>>>>         case ODP_LOG_ABORT: \
>>>>                 fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \
>>>>                 __LINE__, __func__, ##__VA_ARGS__); \
>>>> @@ -111,6 +116,13 @@ do { \
>>>>  } while (0)
>>>>
>>>>  /**
>>>> + * Printing macro, which prints output when the application
>>>> + * calls one of the ODP APIs specifically for dumping internal data.
>>>> + */
>>>> +#define ODP_PRI(fmt, ...) \
>>>> +               ODP_LOG(ODP_LOG_PRI, fmt, ##__VA_ARGS__)
>>>> +
>>>> +/**
>>>>   * Debug printing macro, which prints output when DEBUG flag is set.
>>>>   */
>>>>  #define ODP_DBG(fmt, ...) \
>>>> diff --git a/platform/linux-generic/odp_buffer.c
>>>> b/platform/linux-generic/odp_buffer.c
>>>> index e54e0e7..2ee0461 100644
>>>> --- a/platform/linux-generic/odp_buffer.c
>>>> +++ b/platform/linux-generic/odp_buffer.c
>>>> @@ -52,7 +52,7 @@ int odp_buffer_snprint(char *str, size_t n,
>>>> odp_buffer_t buf)
>>>>         int len = 0;
>>>>
>>>>         if (!odp_buffer_is_valid(buf)) {
>>>> -               printf("Buffer is not valid.\n");
>>>> +               ODP_PRI("Buffer is not valid.\n");
>>>>                 return len;
>>>>         }
>>>>
>>>> @@ -98,7 +98,7 @@ void odp_buffer_print(odp_buffer_t buf)
>>>>         len = odp_buffer_snprint(str, max_len-1, buf);
>>>>         str[len] = 0;
>>>>
>>>> -       printf("\n%s\n", str);
>>>> +       ODP_PRI("\n%s\n", str);
>>>>  }
>>>>
>>>>  void odp_buffer_copy_scatter(odp_buffer_t buf_dst, odp_buffer_t
>>>> buf_src)
>>>> diff --git a/platform/linux-generic/odp_buffer_pool.c
>>>> b/platform/linux-generic/odp_buffer_pool.c
>>>> index a48d7d6..3555166 100644
>>>> --- a/platform/linux-generic/odp_buffer_pool.c
>>>> +++ b/platform/linux-generic/odp_buffer_pool.c
>>>> @@ -523,20 +523,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>>>> pool_hdl)
>>>>         pool_id = pool_handle_to_index(pool_hdl);
>>>>         pool    = get_pool_entry(pool_id);
>>>>
>>>> -       printf("Pool info\n");
>>>> -       printf("---------\n");
>>>> -       printf("  pool          %i\n",           pool->s.pool_hdl);
>>>> -       printf("  name          %s\n",           pool->s.name);
>>>> -       printf("  pool base     %p\n",
>>>>  pool->s.pool_base_addr);
>>>> -       printf("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
>>>> -       printf("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
>>>> -       printf("  buf size      %zu\n",          pool->s.user_size);
>>>> -       printf("  buf align     %zu\n",          pool->s.user_align);
>>>> -       printf("  hdr size      %zu\n",          pool->s.hdr_size);
>>>> -       printf("  alloc size    %zu\n",          pool->s.buf_size);
>>>> -       printf("  offset to hdr %zu\n",          pool->s.buf_offset);
>>>> -       printf("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
>>>> -       printf("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>>>> +       ODP_PRI("Pool info\n");
>>>> +       ODP_PRI("---------\n");
>>>> +       ODP_PRI("  pool          %i\n",           pool->s.pool_hdl);
>>>> +       ODP_PRI("  name          %s\n",           pool->s.name);
>>>> +       ODP_PRI("  pool base     %p\n",
>>>>  pool->s.pool_base_addr);
>>>> +       ODP_PRI("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
>>>> +       ODP_PRI("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
>>>> +       ODP_PRI("  buf size      %zu\n",          pool->s.user_size);
>>>> +       ODP_PRI("  buf align     %zu\n",          pool->s.user_align);
>>>> +       ODP_PRI("  hdr size      %zu\n",          pool->s.hdr_size);
>>>> +       ODP_PRI("  alloc size    %zu\n",          pool->s.buf_size);
>>>> +       ODP_PRI("  offset to hdr %zu\n",          pool->s.buf_offset);
>>>> +       ODP_PRI("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
>>>> +       ODP_PRI("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>>>>
>>>>         /* first chunk */
>>>>         chunk_hdr = pool->s.head;
>>>> @@ -546,7 +546,7 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>>>> pool_hdl)
>>>>                 return;
>>>>         }
>>>>
>>>> -       printf("\n  First chunk\n");
>>>> +       ODP_PRI("\n  First chunk\n");
>>>>
>>>>         for (i = 0; i < chunk_hdr->chunk.num_bufs - 1; i++) {
>>>>                 uint32_t index;
>>>> @@ -555,20 +555,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>>>> pool_hdl)
>>>>                 index = chunk_hdr->chunk.buf_index[i];
>>>>                 hdr   = index_to_hdr(pool, index);
>>>>
>>>> -               printf("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr,
>>>> index);
>>>> +               ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr,
>>>> index);
>>>>         }
>>>>
>>>> -       printf("  [%i] addr %p, id %"PRIu32"\n", i,
>>>> chunk_hdr->buf_hdr.addr,
>>>> -              chunk_hdr->buf_hdr.index);
>>>> +       ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i,
>>>> chunk_hdr->buf_hdr.addr,
>>>> +               chunk_hdr->buf_hdr.index);
>>>>
>>>>         /* next chunk */
>>>>         chunk_hdr = next_chunk(pool, chunk_hdr);
>>>>
>>>>         if (chunk_hdr) {
>>>> -               printf("  Next chunk\n");
>>>> -               printf("  addr %p, id %"PRIu32"\n",
>>>> chunk_hdr->buf_hdr.addr,
>>>> -                      chunk_hdr->buf_hdr.index);
>>>> +               ODP_PRI("  Next chunk\n");
>>>> +               ODP_PRI("  addr %p, id %"PRIu32"\n",
>>>> chunk_hdr->buf_hdr.addr,
>>>> +                       chunk_hdr->buf_hdr.index);
>>>>         }
>>>>
>>>> -       printf("\n");
>>>> +       ODP_PRI("\n");
>>>>  }
>>>> diff --git a/platform/linux-generic/odp_packet.c
>>>> b/platform/linux-generic/odp_packet.c
>>>> index 82ea879..7fe9ed0 100644
>>>> --- a/platform/linux-generic/odp_packet.c
>>>> +++ b/platform/linux-generic/odp_packet.c
>>>> @@ -346,7 +346,7 @@ void odp_packet_print(odp_packet_t pkt)
>>>>                         "  input        %u\n", hdr->input);
>>>>         str[len] = '\0';
>>>>
>>>> -       printf("\n%s\n", str);
>>>> +       ODP_PRI("\n%s\n", str);
>>>>  }
>>>>
>>>>  int odp_packet_copy(odp_packet_t pkt_dst, odp_packet_t pkt_src)
>>>> diff --git a/platform/linux-generic/odp_shared_memory.c
>>>> b/platform/linux-generic/odp_shared_memory.c
>>>> index 24a5d60..cd4415d 100644
>>>> --- a/platform/linux-generic/odp_shared_memory.c
>>>> +++ b/platform/linux-generic/odp_shared_memory.c
>>>> @@ -285,14 +285,14 @@ void odp_shm_print_all(void)
>>>>  {
>>>>         int i;
>>>>
>>>> -       printf("\nShared memory\n");
>>>> -       printf("--------------\n");
>>>> -       printf("  page size:      %"PRIu64" kB\n", odp_sys_page_size()
>>>> / 1024);
>>>> -       printf("  huge page size: %"PRIu64" kB\n",
>>>> -              odp_sys_huge_page_size() / 1024);
>>>> -       printf("\n");
>>>> +       ODP_PRI("\nShared memory\n");
>>>> +       ODP_PRI("--------------\n");
>>>> +       ODP_PRI("  page size:      %"PRIu64" kB\n", odp_sys_page_size()
>>>> / 1024);
>>>> +       ODP_PRI("  huge page size: %"PRIu64" kB\n",
>>>> +               odp_sys_huge_page_size() / 1024);
>>>> +       ODP_PRI("\n");
>>>>
>>>> -       printf("  id name                       kB align huge addr\n");
>>>> +       ODP_PRI("  id name                       kB align huge addr\n");
>>>>
>>>>         for (i = 0; i < ODP_SHM_NUM_BLOCKS; i++) {
>>>>                 odp_shm_block_t *block;
>>>> @@ -300,15 +300,15 @@ void odp_shm_print_all(void)
>>>>                 block = &odp_shm_tbl->block[i];
>>>>
>>>>                 if (block->addr) {
>>>> -                       printf("  %2i %-24s %4"PRIu64"  %4"PRIu64" %2c
>>>>  %p\n",
>>>> -                              i,
>>>> -                              block->name,
>>>> -                              block->size/1024,
>>>> -                              block->align,
>>>> -                              (block->huge ? '*' : ' '),
>>>> -                              block->addr);
>>>> +                       ODP_PRI("  %2i %-24s %4"PRIu64"  %4"PRIu64"
>>>> %2c   %p\n",
>>>> +                               i,
>>>> +                               block->name,
>>>> +                               block->size/1024,
>>>> +                               block->align,
>>>> +                               (block->huge ? '*' : ' '),
>>>> +                               block->addr);
>>>>                 }
>>>>         }
>>>>
>>>> -       printf("\n");
>>>> +       ODP_PRI("\n");
>>>>  }
>>>> --
>>>> 2.1.0
>>>>
>>>>
>>>> _______________________________________________
>>>> lng-odp mailing list
>>>> lng-odp@lists.linaro.org
>>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>>>
>>>
>>>
>>
>>
>> --
>> *Mike Holmes*
>> Linaro  Sr Technical Manager
>> LNG - ODP
>>
>
>

Bill Fischofer Nov. 18, 2014, 7:23 p.m. UTC | #5

ODP_SAY()?

On Tue, Nov 18, 2014 at 1:22 PM, Mike Holmes <mike.holmes@linaro.org> wrote:

>
>
> On 18 November 2014 14:04, Bill Fischofer <bill.fischofer@linaro.org>
> wrote:
>
>> The standard abbreviation for PRINT would be PRT, but that's easily
>> typo'd with PTR, which is the universal standard abbreviation for pointer.
>>
>> Why not just ODP_LOG() then?
>>
>
> That is already used, that is what all the functions funnel into, so to
> use it we have to do some renaming of ODP_LOG to free it up for use in
> place of ODP_PRI
> ODP_LOG is the root and is what should be replaceable by the application
> as it stands, but that patch has not gone in yet.
>
>
>> Torturing the syntax to comply with a procrustean rule on line length
>> points to a bigger issue with the 'standard' we allowed ourself to get
>> saddled with, but that's beyond the scope of this patch.  :)
>>
>
> :)
>
>
>>
>>
>>
>> On Tue, Nov 18, 2014 at 12:56 PM, Mike Holmes <mike.holmes@linaro.org>
>> wrote:
>>
>>>
>>>
>>> On 18 November 2014 13:50, Bill Fischofer <bill.fischofer@linaro.org>
>>> wrote:
>>>
>>>> PRI is neither a standard nor obvious abbreviation for PRINT.  Why not
>>>> just call it ODP_PRINT()?  Is saving two characters worth the confusion?
>>>>
>>>
>>> I thought it was, here is my rational.
>>>
>>> Because the length difference generates all sorts of manual formatting
>>> issues if you swap it between ODP_PRINT ODP_DBG, for example some lines pop
>>> over 80 chars, the parentheses no longer line up etc
>>> I experienced this when creating in the patches of the examples and the
>>> test to remove their use of of ODP_DBG.
>>>
>>> Also 80 chars is so few and with print strings being harder to break
>>> neatly over lines I felt the truncation helped there also.
>>>
>>> But I am not wedded to ODP_PRI, I can change it if there is consensus.
>>>
>>>
>>>>
>>>> Bill
>>>>
>>>> On Tue, Nov 18, 2014 at 12:30 PM, Mike Holmes <mike.holmes@linaro.org>
>>>> wrote:
>>>>
>>>>> Current ODP APIs that print internal data on demand for the
>>>>> application do so
>>>>> via  printf.
>>>>>
>>>>> Introduce ODP_PRI so that APIs that produce output may be channeled
>>>>> though
>>>>> ODP_LOG to match the other logging types.
>>>>>
>>>>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
>>>>> ---
>>>>>
>>>>> This patch does not impact the behaviour of any current example or
>>>>> test.
>>>>>
>>>>> A follow on should implement the application replaceable ODP_LOG
>>>>> function
>>>>> via weak linking.
>>>>>
>>>>>  platform/linux-generic/include/api/odp_debug.h | 14 +++++++-
>>>>>  platform/linux-generic/odp_buffer.c            |  4 +--
>>>>>  platform/linux-generic/odp_buffer_pool.c       | 44
>>>>> +++++++++++++-------------
>>>>>  platform/linux-generic/odp_packet.c            |  2 +-
>>>>>  platform/linux-generic/odp_shared_memory.c     | 30 +++++++++---------
>>>>>  5 files changed, 53 insertions(+), 41 deletions(-)
>>>>>
>>>>> diff --git a/platform/linux-generic/include/api/odp_debug.h
>>>>> b/platform/linux-generic/include/api/odp_debug.h
>>>>> index c9b2edd..1a6fbda 100644
>>>>> --- a/platform/linux-generic/include/api/odp_debug.h
>>>>> +++ b/platform/linux-generic/include/api/odp_debug.h
>>>>> @@ -76,7 +76,8 @@ typedef enum odp_log_level {
>>>>>         ODP_LOG_DBG,
>>>>>         ODP_LOG_ERR,
>>>>>         ODP_LOG_UNIMPLEMENTED,
>>>>> -       ODP_LOG_ABORT
>>>>> +       ODP_LOG_ABORT,
>>>>> +       ODP_LOG_PRI,
>>>>>  } odp_log_level_e;
>>>>>
>>>>>  /**
>>>>> @@ -94,6 +95,10 @@ do { \
>>>>>                         fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>>>>>                         __LINE__, __func__, ##__VA_ARGS__); \
>>>>>                 break; \
>>>>> +       case ODP_LOG_PRI: \
>>>>> +               fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>>>>> +               __LINE__, __func__, ##__VA_ARGS__); \
>>>>> +               break; \
>>>>>         case ODP_LOG_ABORT: \
>>>>>                 fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \
>>>>>                 __LINE__, __func__, ##__VA_ARGS__); \
>>>>> @@ -111,6 +116,13 @@ do { \
>>>>>  } while (0)
>>>>>
>>>>>  /**
>>>>> + * Printing macro, which prints output when the application
>>>>> + * calls one of the ODP APIs specifically for dumping internal data.
>>>>> + */
>>>>> +#define ODP_PRI(fmt, ...) \
>>>>> +               ODP_LOG(ODP_LOG_PRI, fmt, ##__VA_ARGS__)
>>>>> +
>>>>> +/**
>>>>>   * Debug printing macro, which prints output when DEBUG flag is set.
>>>>>   */
>>>>>  #define ODP_DBG(fmt, ...) \
>>>>> diff --git a/platform/linux-generic/odp_buffer.c
>>>>> b/platform/linux-generic/odp_buffer.c
>>>>> index e54e0e7..2ee0461 100644
>>>>> --- a/platform/linux-generic/odp_buffer.c
>>>>> +++ b/platform/linux-generic/odp_buffer.c
>>>>> @@ -52,7 +52,7 @@ int odp_buffer_snprint(char *str, size_t n,
>>>>> odp_buffer_t buf)
>>>>>         int len = 0;
>>>>>
>>>>>         if (!odp_buffer_is_valid(buf)) {
>>>>> -               printf("Buffer is not valid.\n");
>>>>> +               ODP_PRI("Buffer is not valid.\n");
>>>>>                 return len;
>>>>>         }
>>>>>
>>>>> @@ -98,7 +98,7 @@ void odp_buffer_print(odp_buffer_t buf)
>>>>>         len = odp_buffer_snprint(str, max_len-1, buf);
>>>>>         str[len] = 0;
>>>>>
>>>>> -       printf("\n%s\n", str);
>>>>> +       ODP_PRI("\n%s\n", str);
>>>>>  }
>>>>>
>>>>>  void odp_buffer_copy_scatter(odp_buffer_t buf_dst, odp_buffer_t
>>>>> buf_src)
>>>>> diff --git a/platform/linux-generic/odp_buffer_pool.c
>>>>> b/platform/linux-generic/odp_buffer_pool.c
>>>>> index a48d7d6..3555166 100644
>>>>> --- a/platform/linux-generic/odp_buffer_pool.c
>>>>> +++ b/platform/linux-generic/odp_buffer_pool.c
>>>>> @@ -523,20 +523,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>>>>> pool_hdl)
>>>>>         pool_id = pool_handle_to_index(pool_hdl);
>>>>>         pool    = get_pool_entry(pool_id);
>>>>>
>>>>> -       printf("Pool info\n");
>>>>> -       printf("---------\n");
>>>>> -       printf("  pool          %i\n",           pool->s.pool_hdl);
>>>>> -       printf("  name          %s\n",           pool->s.name);
>>>>> -       printf("  pool base     %p\n",
>>>>>  pool->s.pool_base_addr);
>>>>> -       printf("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
>>>>> -       printf("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
>>>>> -       printf("  buf size      %zu\n",          pool->s.user_size);
>>>>> -       printf("  buf align     %zu\n",          pool->s.user_align);
>>>>> -       printf("  hdr size      %zu\n",          pool->s.hdr_size);
>>>>> -       printf("  alloc size    %zu\n",          pool->s.buf_size);
>>>>> -       printf("  offset to hdr %zu\n",          pool->s.buf_offset);
>>>>> -       printf("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
>>>>> -       printf("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>>>>> +       ODP_PRI("Pool info\n");
>>>>> +       ODP_PRI("---------\n");
>>>>> +       ODP_PRI("  pool          %i\n",           pool->s.pool_hdl);
>>>>> +       ODP_PRI("  name          %s\n",           pool->s.name);
>>>>> +       ODP_PRI("  pool base     %p\n",
>>>>>  pool->s.pool_base_addr);
>>>>> +       ODP_PRI("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
>>>>> +       ODP_PRI("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
>>>>> +       ODP_PRI("  buf size      %zu\n",          pool->s.user_size);
>>>>> +       ODP_PRI("  buf align     %zu\n",          pool->s.user_align);
>>>>> +       ODP_PRI("  hdr size      %zu\n",          pool->s.hdr_size);
>>>>> +       ODP_PRI("  alloc size    %zu\n",          pool->s.buf_size);
>>>>> +       ODP_PRI("  offset to hdr %zu\n",          pool->s.buf_offset);
>>>>> +       ODP_PRI("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
>>>>> +       ODP_PRI("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>>>>>
>>>>>         /* first chunk */
>>>>>         chunk_hdr = pool->s.head;
>>>>> @@ -546,7 +546,7 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>>>>> pool_hdl)
>>>>>                 return;
>>>>>         }
>>>>>
>>>>> -       printf("\n  First chunk\n");
>>>>> +       ODP_PRI("\n  First chunk\n");
>>>>>
>>>>>         for (i = 0; i < chunk_hdr->chunk.num_bufs - 1; i++) {
>>>>>                 uint32_t index;
>>>>> @@ -555,20 +555,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>>>>> pool_hdl)
>>>>>                 index = chunk_hdr->chunk.buf_index[i];
>>>>>                 hdr   = index_to_hdr(pool, index);
>>>>>
>>>>> -               printf("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr,
>>>>> index);
>>>>> +               ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i,
>>>>> hdr->addr, index);
>>>>>         }
>>>>>
>>>>> -       printf("  [%i] addr %p, id %"PRIu32"\n", i,
>>>>> chunk_hdr->buf_hdr.addr,
>>>>> -              chunk_hdr->buf_hdr.index);
>>>>> +       ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i,
>>>>> chunk_hdr->buf_hdr.addr,
>>>>> +               chunk_hdr->buf_hdr.index);
>>>>>
>>>>>         /* next chunk */
>>>>>         chunk_hdr = next_chunk(pool, chunk_hdr);
>>>>>
>>>>>         if (chunk_hdr) {
>>>>> -               printf("  Next chunk\n");
>>>>> -               printf("  addr %p, id %"PRIu32"\n",
>>>>> chunk_hdr->buf_hdr.addr,
>>>>> -                      chunk_hdr->buf_hdr.index);
>>>>> +               ODP_PRI("  Next chunk\n");
>>>>> +               ODP_PRI("  addr %p, id %"PRIu32"\n",
>>>>> chunk_hdr->buf_hdr.addr,
>>>>> +                       chunk_hdr->buf_hdr.index);
>>>>>         }
>>>>>
>>>>> -       printf("\n");
>>>>> +       ODP_PRI("\n");
>>>>>  }
>>>>> diff --git a/platform/linux-generic/odp_packet.c
>>>>> b/platform/linux-generic/odp_packet.c
>>>>> index 82ea879..7fe9ed0 100644
>>>>> --- a/platform/linux-generic/odp_packet.c
>>>>> +++ b/platform/linux-generic/odp_packet.c
>>>>> @@ -346,7 +346,7 @@ void odp_packet_print(odp_packet_t pkt)
>>>>>                         "  input        %u\n", hdr->input);
>>>>>         str[len] = '\0';
>>>>>
>>>>> -       printf("\n%s\n", str);
>>>>> +       ODP_PRI("\n%s\n", str);
>>>>>  }
>>>>>
>>>>>  int odp_packet_copy(odp_packet_t pkt_dst, odp_packet_t pkt_src)
>>>>> diff --git a/platform/linux-generic/odp_shared_memory.c
>>>>> b/platform/linux-generic/odp_shared_memory.c
>>>>> index 24a5d60..cd4415d 100644
>>>>> --- a/platform/linux-generic/odp_shared_memory.c
>>>>> +++ b/platform/linux-generic/odp_shared_memory.c
>>>>> @@ -285,14 +285,14 @@ void odp_shm_print_all(void)
>>>>>  {
>>>>>         int i;
>>>>>
>>>>> -       printf("\nShared memory\n");
>>>>> -       printf("--------------\n");
>>>>> -       printf("  page size:      %"PRIu64" kB\n", odp_sys_page_size()
>>>>> / 1024);
>>>>> -       printf("  huge page size: %"PRIu64" kB\n",
>>>>> -              odp_sys_huge_page_size() / 1024);
>>>>> -       printf("\n");
>>>>> +       ODP_PRI("\nShared memory\n");
>>>>> +       ODP_PRI("--------------\n");
>>>>> +       ODP_PRI("  page size:      %"PRIu64" kB\n",
>>>>> odp_sys_page_size() / 1024);
>>>>> +       ODP_PRI("  huge page size: %"PRIu64" kB\n",
>>>>> +               odp_sys_huge_page_size() / 1024);
>>>>> +       ODP_PRI("\n");
>>>>>
>>>>> -       printf("  id name                       kB align huge addr\n");
>>>>> +       ODP_PRI("  id name                       kB align huge
>>>>> addr\n");
>>>>>
>>>>>         for (i = 0; i < ODP_SHM_NUM_BLOCKS; i++) {
>>>>>                 odp_shm_block_t *block;
>>>>> @@ -300,15 +300,15 @@ void odp_shm_print_all(void)
>>>>>                 block = &odp_shm_tbl->block[i];
>>>>>
>>>>>                 if (block->addr) {
>>>>> -                       printf("  %2i %-24s %4"PRIu64"  %4"PRIu64"
>>>>> %2c   %p\n",
>>>>> -                              i,
>>>>> -                              block->name,
>>>>> -                              block->size/1024,
>>>>> -                              block->align,
>>>>> -                              (block->huge ? '*' : ' '),
>>>>> -                              block->addr);
>>>>> +                       ODP_PRI("  %2i %-24s %4"PRIu64"  %4"PRIu64"
>>>>> %2c   %p\n",
>>>>> +                               i,
>>>>> +                               block->name,
>>>>> +                               block->size/1024,
>>>>> +                               block->align,
>>>>> +                               (block->huge ? '*' : ' '),
>>>>> +                               block->addr);
>>>>>                 }
>>>>>         }
>>>>>
>>>>> -       printf("\n");
>>>>> +       ODP_PRI("\n");
>>>>>  }
>>>>> --
>>>>> 2.1.0
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> lng-odp mailing list
>>>>> lng-odp@lists.linaro.org
>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> *Mike Holmes*
>>> Linaro  Sr Technical Manager
>>> LNG - ODP
>>>
>>
>>
>
>
> --
> *Mike Holmes*
> Linaro  Sr Technical Manager
> LNG - ODP
>

Maxim Uvarov Nov. 18, 2014, 10:57 p.m. UTC | #6

I think we are going to have huge number of functions which do message 
print.

Can we use single function for that? Might be

odp_pr(enum level, fmt, ##__VA_ARGS__);
This is not needed to be macro, it's ok to be function.

Also one comment bellow about that patch.
Maxim.


On 11/18/2014 09:30 PM, Mike Holmes wrote:
> Current ODP APIs that print internal data on demand for the application do so
> via  printf.
>
> Introduce ODP_PRI so that APIs that produce output may be channeled though
> ODP_LOG to match the other logging types.
>
> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
> ---
>
> This patch does not impact the behaviour of any current example or test.
>
> A follow on should implement the application replaceable ODP_LOG function
> via weak linking.
>
>   platform/linux-generic/include/api/odp_debug.h | 14 +++++++-
>   platform/linux-generic/odp_buffer.c            |  4 +--
>   platform/linux-generic/odp_buffer_pool.c       | 44 +++++++++++++-------------
>   platform/linux-generic/odp_packet.c            |  2 +-
>   platform/linux-generic/odp_shared_memory.c     | 30 +++++++++---------
>   5 files changed, 53 insertions(+), 41 deletions(-)
>
> diff --git a/platform/linux-generic/include/api/odp_debug.h b/platform/linux-generic/include/api/odp_debug.h
> index c9b2edd..1a6fbda 100644
> --- a/platform/linux-generic/include/api/odp_debug.h
> +++ b/platform/linux-generic/include/api/odp_debug.h
> @@ -76,7 +76,8 @@ typedef enum odp_log_level {
>   	ODP_LOG_DBG,
>   	ODP_LOG_ERR,
>   	ODP_LOG_UNIMPLEMENTED,
> -	ODP_LOG_ABORT
> +	ODP_LOG_ABORT,
> +	ODP_LOG_PRI,
>   } odp_log_level_e;
>   
>   /**
> @@ -94,6 +95,10 @@ do { \
>   			fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>   			__LINE__, __func__, ##__VA_ARGS__); \
>   		break; \
> +	case ODP_LOG_PRI: \
> +		fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
> +		__LINE__, __func__, ##__VA_ARGS__); \
> +		break; \

So you also changing print from stdout to stderr. Reason?

>   	case ODP_LOG_ABORT: \
>   		fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \
>   		__LINE__, __func__, ##__VA_ARGS__); \
> @@ -111,6 +116,13 @@ do { \
>   } while (0)
>   
>   /**
> + * Printing macro, which prints output when the application
> + * calls one of the ODP APIs specifically for dumping internal data.
> + */
> +#define ODP_PRI(fmt, ...) \
> +		ODP_LOG(ODP_LOG_PRI, fmt, ##__VA_ARGS__)
> +
> +/**
>    * Debug printing macro, which prints output when DEBUG flag is set.
>    */
>   #define ODP_DBG(fmt, ...) \
> diff --git a/platform/linux-generic/odp_buffer.c b/platform/linux-generic/odp_buffer.c
> index e54e0e7..2ee0461 100644
> --- a/platform/linux-generic/odp_buffer.c
> +++ b/platform/linux-generic/odp_buffer.c
> @@ -52,7 +52,7 @@ int odp_buffer_snprint(char *str, size_t n, odp_buffer_t buf)
>   	int len = 0;
>   
>   	if (!odp_buffer_is_valid(buf)) {
> -		printf("Buffer is not valid.\n");
> +		ODP_PRI("Buffer is not valid.\n");
>   		return len;
>   	}
>   
> @@ -98,7 +98,7 @@ void odp_buffer_print(odp_buffer_t buf)
>   	len = odp_buffer_snprint(str, max_len-1, buf);
>   	str[len] = 0;
>   
> -	printf("\n%s\n", str);
> +	ODP_PRI("\n%s\n", str);
>   }
>   
>   void odp_buffer_copy_scatter(odp_buffer_t buf_dst, odp_buffer_t buf_src)
> diff --git a/platform/linux-generic/odp_buffer_pool.c b/platform/linux-generic/odp_buffer_pool.c
> index a48d7d6..3555166 100644
> --- a/platform/linux-generic/odp_buffer_pool.c
> +++ b/platform/linux-generic/odp_buffer_pool.c
> @@ -523,20 +523,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t pool_hdl)
>   	pool_id = pool_handle_to_index(pool_hdl);
>   	pool    = get_pool_entry(pool_id);
>   
> -	printf("Pool info\n");
> -	printf("---------\n");
> -	printf("  pool          %i\n",           pool->s.pool_hdl);
> -	printf("  name          %s\n",           pool->s.name);
> -	printf("  pool base     %p\n",           pool->s.pool_base_addr);
> -	printf("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
> -	printf("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
> -	printf("  buf size      %zu\n",          pool->s.user_size);
> -	printf("  buf align     %zu\n",          pool->s.user_align);
> -	printf("  hdr size      %zu\n",          pool->s.hdr_size);
> -	printf("  alloc size    %zu\n",          pool->s.buf_size);
> -	printf("  offset to hdr %zu\n",          pool->s.buf_offset);
> -	printf("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
> -	printf("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
> +	ODP_PRI("Pool info\n");
> +	ODP_PRI("---------\n");
> +	ODP_PRI("  pool          %i\n",           pool->s.pool_hdl);
> +	ODP_PRI("  name          %s\n",           pool->s.name);
> +	ODP_PRI("  pool base     %p\n",           pool->s.pool_base_addr);
> +	ODP_PRI("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
> +	ODP_PRI("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
> +	ODP_PRI("  buf size      %zu\n",          pool->s.user_size);
> +	ODP_PRI("  buf align     %zu\n",          pool->s.user_align);
> +	ODP_PRI("  hdr size      %zu\n",          pool->s.hdr_size);
> +	ODP_PRI("  alloc size    %zu\n",          pool->s.buf_size);
> +	ODP_PRI("  offset to hdr %zu\n",          pool->s.buf_offset);
> +	ODP_PRI("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
> +	ODP_PRI("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>   
>   	/* first chunk */
>   	chunk_hdr = pool->s.head;
> @@ -546,7 +546,7 @@ void odp_buffer_pool_print(odp_buffer_pool_t pool_hdl)
>   		return;
>   	}
>   
> -	printf("\n  First chunk\n");
> +	ODP_PRI("\n  First chunk\n");
>   
>   	for (i = 0; i < chunk_hdr->chunk.num_bufs - 1; i++) {
>   		uint32_t index;
> @@ -555,20 +555,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t pool_hdl)
>   		index = chunk_hdr->chunk.buf_index[i];
>   		hdr   = index_to_hdr(pool, index);
>   
> -		printf("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr, index);
> +		ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr, index);
>   	}
>   
> -	printf("  [%i] addr %p, id %"PRIu32"\n", i, chunk_hdr->buf_hdr.addr,
> -	       chunk_hdr->buf_hdr.index);
> +	ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i, chunk_hdr->buf_hdr.addr,
> +	        chunk_hdr->buf_hdr.index);
>   
>   	/* next chunk */
>   	chunk_hdr = next_chunk(pool, chunk_hdr);
>   
>   	if (chunk_hdr) {
> -		printf("  Next chunk\n");
> -		printf("  addr %p, id %"PRIu32"\n", chunk_hdr->buf_hdr.addr,
> -		       chunk_hdr->buf_hdr.index);
> +		ODP_PRI("  Next chunk\n");
> +		ODP_PRI("  addr %p, id %"PRIu32"\n", chunk_hdr->buf_hdr.addr,
> +		        chunk_hdr->buf_hdr.index);
>   	}
>   
> -	printf("\n");
> +	ODP_PRI("\n");
>   }
> diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-generic/odp_packet.c
> index 82ea879..7fe9ed0 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -346,7 +346,7 @@ void odp_packet_print(odp_packet_t pkt)
>   			"  input        %u\n", hdr->input);
>   	str[len] = '\0';
>   
> -	printf("\n%s\n", str);
> +	ODP_PRI("\n%s\n", str);
>   }
>   
>   int odp_packet_copy(odp_packet_t pkt_dst, odp_packet_t pkt_src)
> diff --git a/platform/linux-generic/odp_shared_memory.c b/platform/linux-generic/odp_shared_memory.c
> index 24a5d60..cd4415d 100644
> --- a/platform/linux-generic/odp_shared_memory.c
> +++ b/platform/linux-generic/odp_shared_memory.c
> @@ -285,14 +285,14 @@ void odp_shm_print_all(void)
>   {
>   	int i;
>   
> -	printf("\nShared memory\n");
> -	printf("--------------\n");
> -	printf("  page size:      %"PRIu64" kB\n", odp_sys_page_size() / 1024);
> -	printf("  huge page size: %"PRIu64" kB\n",
> -	       odp_sys_huge_page_size() / 1024);
> -	printf("\n");
> +	ODP_PRI("\nShared memory\n");
> +	ODP_PRI("--------------\n");
> +	ODP_PRI("  page size:      %"PRIu64" kB\n", odp_sys_page_size() / 1024);
> +	ODP_PRI("  huge page size: %"PRIu64" kB\n",
> +	        odp_sys_huge_page_size() / 1024);
> +	ODP_PRI("\n");
>   
> -	printf("  id name                       kB align huge addr\n");
> +	ODP_PRI("  id name                       kB align huge addr\n");
>   
>   	for (i = 0; i < ODP_SHM_NUM_BLOCKS; i++) {
>   		odp_shm_block_t *block;
> @@ -300,15 +300,15 @@ void odp_shm_print_all(void)
>   		block = &odp_shm_tbl->block[i];
>   
>   		if (block->addr) {
> -			printf("  %2i %-24s %4"PRIu64"  %4"PRIu64" %2c   %p\n",
> -			       i,
> -			       block->name,
> -			       block->size/1024,
> -			       block->align,
> -			       (block->huge ? '*' : ' '),
> -			       block->addr);
> +			ODP_PRI("  %2i %-24s %4"PRIu64"  %4"PRIu64" %2c   %p\n",
> +			        i,
> +			        block->name,
> +			        block->size/1024,
> +			        block->align,
> +			        (block->huge ? '*' : ' '),
> +			        block->addr);
>   		}
>   	}
>   
> -	printf("\n");
> +	ODP_PRI("\n");
>   }

Taras Kondratiuk Nov. 19, 2014, 8:47 a.m. UTC | #7

On 11/18/2014 09:23 PM, Bill Fischofer wrote:
> ODP_SAY()?

A few more:
LOG_APP
LOG_USR

Normally odp_*_print() functions will be used for debugging.
Maybe they can be printed with LOG_DBG level?

Mike Holmes Nov. 19, 2014, 3:32 p.m. UTC | #8

On 19 November 2014 03:47, Taras Kondratiuk <taras.kondratiuk@linaro.org>
wrote:

> On 11/18/2014 09:23 PM, Bill Fischofer wrote:
>
>> ODP_SAY()?
>>
>
> A few more:
> LOG_APP
> LOG_USR
>
> Normally odp_*_print() functions will be used for debugging.
> Maybe they can be printed with LOG_DBG level?
>

This is specifically not a debug level it is an application called print of
internal information, the only issue with its use of printf is that it
cannot be redirected.

Mike Holmes Nov. 19, 2014, 3:35 p.m. UTC | #9

On 18 November 2014 17:57, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:

> I think we are going to have huge number of functions which do message
> print.
>

I hope not, just cases where the implementation needs to return data for
application logs to use

>
> Can we use single function for that? Might be
>
> odp_pr(enum level, fmt, ##__VA_ARGS__);
> This is not needed to be macro, it's ok to be function.
>
> Also one comment bellow about that patch.
> Maxim.


Lets not make up yet another log, it is ALL going via what ODP_LOG becomes,
I like the name ODP_PR out of all the suggestions so far however


>
>
>
> On 11/18/2014 09:30 PM, Mike Holmes wrote:
>
>> Current ODP APIs that print internal data on demand for the application
>> do so
>> via  printf.
>>
>> Introduce ODP_PRI so that APIs that produce output may be channeled though
>> ODP_LOG to match the other logging types.
>>
>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
>> ---
>>
>> This patch does not impact the behaviour of any current example or test.
>>
>> A follow on should implement the application replaceable ODP_LOG function
>> via weak linking.
>>
>>   platform/linux-generic/include/api/odp_debug.h | 14 +++++++-
>>   platform/linux-generic/odp_buffer.c            |  4 +--
>>   platform/linux-generic/odp_buffer_pool.c       | 44
>> +++++++++++++-------------
>>   platform/linux-generic/odp_packet.c            |  2 +-
>>   platform/linux-generic/odp_shared_memory.c     | 30 +++++++++---------
>>   5 files changed, 53 insertions(+), 41 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/api/odp_debug.h
>> b/platform/linux-generic/include/api/odp_debug.h
>> index c9b2edd..1a6fbda 100644
>> --- a/platform/linux-generic/include/api/odp_debug.h
>> +++ b/platform/linux-generic/include/api/odp_debug.h
>> @@ -76,7 +76,8 @@ typedef enum odp_log_level {
>>         ODP_LOG_DBG,
>>         ODP_LOG_ERR,
>>         ODP_LOG_UNIMPLEMENTED,
>> -       ODP_LOG_ABORT
>> +       ODP_LOG_ABORT,
>> +       ODP_LOG_PRI,
>>   } odp_log_level_e;
>>     /**
>> @@ -94,6 +95,10 @@ do { \
>>                         fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>>                         __LINE__, __func__, ##__VA_ARGS__); \
>>                 break; \
>> +       case ODP_LOG_PRI: \
>> +               fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>> +               __LINE__, __func__, ##__VA_ARGS__); \
>> +               break; \
>>
>
> So you also changing print from stdout to stderr. Reason?


No - thank you that is a mistake and I will fix it.

>
>
>          case ODP_LOG_ABORT: \
>>                 fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \
>>                 __LINE__, __func__, ##__VA_ARGS__); \
>> @@ -111,6 +116,13 @@ do { \
>>   } while (0)
>>     /**
>> + * Printing macro, which prints output when the application
>> + * calls one of the ODP APIs specifically for dumping internal data.
>> + */
>> +#define ODP_PRI(fmt, ...) \
>> +               ODP_LOG(ODP_LOG_PRI, fmt, ##__VA_ARGS__)
>> +
>> +/**
>>    * Debug printing macro, which prints output when DEBUG flag is set.
>>    */
>>   #define ODP_DBG(fmt, ...) \
>> diff --git a/platform/linux-generic/odp_buffer.c
>> b/platform/linux-generic/odp_buffer.c
>> index e54e0e7..2ee0461 100644
>> --- a/platform/linux-generic/odp_buffer.c
>> +++ b/platform/linux-generic/odp_buffer.c
>> @@ -52,7 +52,7 @@ int odp_buffer_snprint(char *str, size_t n,
>> odp_buffer_t buf)
>>         int len = 0;
>>         if (!odp_buffer_is_valid(buf)) {
>> -               printf("Buffer is not valid.\n");
>> +               ODP_PRI("Buffer is not valid.\n");
>>                 return len;
>>         }
>>   @@ -98,7 +98,7 @@ void odp_buffer_print(odp_buffer_t buf)
>>         len = odp_buffer_snprint(str, max_len-1, buf);
>>         str[len] = 0;
>>   -     printf("\n%s\n", str);
>> +       ODP_PRI("\n%s\n", str);
>>   }
>>     void odp_buffer_copy_scatter(odp_buffer_t buf_dst, odp_buffer_t
>> buf_src)
>> diff --git a/platform/linux-generic/odp_buffer_pool.c
>> b/platform/linux-generic/odp_buffer_pool.c
>> index a48d7d6..3555166 100644
>> --- a/platform/linux-generic/odp_buffer_pool.c
>> +++ b/platform/linux-generic/odp_buffer_pool.c
>> @@ -523,20 +523,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>> pool_hdl)
>>         pool_id = pool_handle_to_index(pool_hdl);
>>         pool    = get_pool_entry(pool_id);
>>   -     printf("Pool info\n");
>> -       printf("---------\n");
>> -       printf("  pool          %i\n",           pool->s.pool_hdl);
>> -       printf("  name          %s\n",           pool->s.name);
>> -       printf("  pool base     %p\n",           pool->s.pool_base_addr);
>> -       printf("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
>> -       printf("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
>> -       printf("  buf size      %zu\n",          pool->s.user_size);
>> -       printf("  buf align     %zu\n",          pool->s.user_align);
>> -       printf("  hdr size      %zu\n",          pool->s.hdr_size);
>> -       printf("  alloc size    %zu\n",          pool->s.buf_size);
>> -       printf("  offset to hdr %zu\n",          pool->s.buf_offset);
>> -       printf("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
>> -       printf("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>> +       ODP_PRI("Pool info\n");
>> +       ODP_PRI("---------\n");
>> +       ODP_PRI("  pool          %i\n",           pool->s.pool_hdl);
>> +       ODP_PRI("  name          %s\n",           pool->s.name);
>> +       ODP_PRI("  pool base     %p\n",           pool->s.pool_base_addr);
>> +       ODP_PRI("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
>> +       ODP_PRI("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
>> +       ODP_PRI("  buf size      %zu\n",          pool->s.user_size);
>> +       ODP_PRI("  buf align     %zu\n",          pool->s.user_align);
>> +       ODP_PRI("  hdr size      %zu\n",          pool->s.hdr_size);
>> +       ODP_PRI("  alloc size    %zu\n",          pool->s.buf_size);
>> +       ODP_PRI("  offset to hdr %zu\n",          pool->s.buf_offset);
>> +       ODP_PRI("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
>> +       ODP_PRI("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>>         /* first chunk */
>>         chunk_hdr = pool->s.head;
>> @@ -546,7 +546,7 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>> pool_hdl)
>>                 return;
>>         }
>>   -     printf("\n  First chunk\n");
>> +       ODP_PRI("\n  First chunk\n");
>>         for (i = 0; i < chunk_hdr->chunk.num_bufs - 1; i++) {
>>                 uint32_t index;
>> @@ -555,20 +555,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>> pool_hdl)
>>                 index = chunk_hdr->chunk.buf_index[i];
>>                 hdr   = index_to_hdr(pool, index);
>>   -             printf("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr,
>> index);
>> +               ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr,
>> index);
>>         }
>>   -     printf("  [%i] addr %p, id %"PRIu32"\n", i,
>> chunk_hdr->buf_hdr.addr,
>> -              chunk_hdr->buf_hdr.index);
>> +       ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i,
>> chunk_hdr->buf_hdr.addr,
>> +               chunk_hdr->buf_hdr.index);
>>         /* next chunk */
>>         chunk_hdr = next_chunk(pool, chunk_hdr);
>>         if (chunk_hdr) {
>> -               printf("  Next chunk\n");
>> -               printf("  addr %p, id %"PRIu32"\n",
>> chunk_hdr->buf_hdr.addr,
>> -                      chunk_hdr->buf_hdr.index);
>> +               ODP_PRI("  Next chunk\n");
>> +               ODP_PRI("  addr %p, id %"PRIu32"\n",
>> chunk_hdr->buf_hdr.addr,
>> +                       chunk_hdr->buf_hdr.index);
>>         }
>>   -     printf("\n");
>> +       ODP_PRI("\n");
>>   }
>> diff --git a/platform/linux-generic/odp_packet.c
>> b/platform/linux-generic/odp_packet.c
>> index 82ea879..7fe9ed0 100644
>> --- a/platform/linux-generic/odp_packet.c
>> +++ b/platform/linux-generic/odp_packet.c
>> @@ -346,7 +346,7 @@ void odp_packet_print(odp_packet_t pkt)
>>                         "  input        %u\n", hdr->input);
>>         str[len] = '\0';
>>   -     printf("\n%s\n", str);
>> +       ODP_PRI("\n%s\n", str);
>>   }
>>     int odp_packet_copy(odp_packet_t pkt_dst, odp_packet_t pkt_src)
>> diff --git a/platform/linux-generic/odp_shared_memory.c
>> b/platform/linux-generic/odp_shared_memory.c
>> index 24a5d60..cd4415d 100644
>> --- a/platform/linux-generic/odp_shared_memory.c
>> +++ b/platform/linux-generic/odp_shared_memory.c
>> @@ -285,14 +285,14 @@ void odp_shm_print_all(void)
>>   {
>>         int i;
>>   -     printf("\nShared memory\n");
>> -       printf("--------------\n");
>> -       printf("  page size:      %"PRIu64" kB\n", odp_sys_page_size() /
>> 1024);
>> -       printf("  huge page size: %"PRIu64" kB\n",
>> -              odp_sys_huge_page_size() / 1024);
>> -       printf("\n");
>> +       ODP_PRI("\nShared memory\n");
>> +       ODP_PRI("--------------\n");
>> +       ODP_PRI("  page size:      %"PRIu64" kB\n", odp_sys_page_size() /
>> 1024);
>> +       ODP_PRI("  huge page size: %"PRIu64" kB\n",
>> +               odp_sys_huge_page_size() / 1024);
>> +       ODP_PRI("\n");
>>   -     printf("  id name                       kB align huge addr\n");
>> +       ODP_PRI("  id name                       kB align huge addr\n");
>>         for (i = 0; i < ODP_SHM_NUM_BLOCKS; i++) {
>>                 odp_shm_block_t *block;
>> @@ -300,15 +300,15 @@ void odp_shm_print_all(void)
>>                 block = &odp_shm_tbl->block[i];
>>                 if (block->addr) {
>> -                       printf("  %2i %-24s %4"PRIu64"  %4"PRIu64" %2c
>>  %p\n",
>> -                              i,
>> -                              block->name,
>> -                              block->size/1024,
>> -                              block->align,
>> -                              (block->huge ? '*' : ' '),
>> -                              block->addr);
>> +                       ODP_PRI("  %2i %-24s %4"PRIu64"  %4"PRIu64" %2c
>>  %p\n",
>> +                               i,
>> +                               block->name,
>> +                               block->size/1024,
>> +                               block->align,
>> +                               (block->huge ? '*' : ' '),
>> +                               block->addr);
>>                 }
>>         }
>>   -     printf("\n");
>> +       ODP_PRI("\n");
>>   }
>>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>

Mike Holmes Nov. 19, 2014, 3:46 p.m. UTC | #10

The only real issue here for this internal always on print capability out
of the library is the name.
I can work with  ODP_PR or ODP_PRINT either is better then ODP_PRI by the
sounds of it.
I think I will go with ODP_PRINT and push v2 after waiting a little while
longer for any other review comments

On 19 November 2014 10:35, Mike Holmes <mike.holmes@linaro.org> wrote:

>
>
> On 18 November 2014 17:57, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
>> I think we are going to have huge number of functions which do message
>> print.
>>
>
> I hope not, just cases where the implementation needs to return data for
> application logs to use
>
>>
>> Can we use single function for that? Might be
>>
>> odp_pr(enum level, fmt, ##__VA_ARGS__);
>> This is not needed to be macro, it's ok to be function.
>>
>> Also one comment bellow about that patch.
>> Maxim.
>
>
> Lets not make up yet another log, it is ALL going via what ODP_LOG
> becomes, I like the name ODP_PR out of all the suggestions so far however
>
>
>>
>>
>>
>> On 11/18/2014 09:30 PM, Mike Holmes wrote:
>>
>>> Current ODP APIs that print internal data on demand for the application
>>> do so
>>> via  printf.
>>>
>>> Introduce ODP_PRI so that APIs that produce output may be channeled
>>> though
>>> ODP_LOG to match the other logging types.
>>>
>>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
>>> ---
>>>
>>> This patch does not impact the behaviour of any current example or test.
>>>
>>> A follow on should implement the application replaceable ODP_LOG function
>>> via weak linking.
>>>
>>>   platform/linux-generic/include/api/odp_debug.h | 14 +++++++-
>>>   platform/linux-generic/odp_buffer.c            |  4 +--
>>>   platform/linux-generic/odp_buffer_pool.c       | 44
>>> +++++++++++++-------------
>>>   platform/linux-generic/odp_packet.c            |  2 +-
>>>   platform/linux-generic/odp_shared_memory.c     | 30 +++++++++---------
>>>   5 files changed, 53 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/platform/linux-generic/include/api/odp_debug.h
>>> b/platform/linux-generic/include/api/odp_debug.h
>>> index c9b2edd..1a6fbda 100644
>>> --- a/platform/linux-generic/include/api/odp_debug.h
>>> +++ b/platform/linux-generic/include/api/odp_debug.h
>>> @@ -76,7 +76,8 @@ typedef enum odp_log_level {
>>>         ODP_LOG_DBG,
>>>         ODP_LOG_ERR,
>>>         ODP_LOG_UNIMPLEMENTED,
>>> -       ODP_LOG_ABORT
>>> +       ODP_LOG_ABORT,
>>> +       ODP_LOG_PRI,
>>>   } odp_log_level_e;
>>>     /**
>>> @@ -94,6 +95,10 @@ do { \
>>>                         fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>>>                         __LINE__, __func__, ##__VA_ARGS__); \
>>>                 break; \
>>> +       case ODP_LOG_PRI: \
>>> +               fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
>>> +               __LINE__, __func__, ##__VA_ARGS__); \
>>> +               break; \
>>>
>>
>> So you also changing print from stdout to stderr. Reason?
>
>
> No - thank you that is a mistake and I will fix it.
>
>>
>>
>>          case ODP_LOG_ABORT: \
>>>                 fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \
>>>                 __LINE__, __func__, ##__VA_ARGS__); \
>>> @@ -111,6 +116,13 @@ do { \
>>>   } while (0)
>>>     /**
>>> + * Printing macro, which prints output when the application
>>> + * calls one of the ODP APIs specifically for dumping internal data.
>>> + */
>>> +#define ODP_PRI(fmt, ...) \
>>> +               ODP_LOG(ODP_LOG_PRI, fmt, ##__VA_ARGS__)
>>> +
>>> +/**
>>>    * Debug printing macro, which prints output when DEBUG flag is set.
>>>    */
>>>   #define ODP_DBG(fmt, ...) \
>>> diff --git a/platform/linux-generic/odp_buffer.c
>>> b/platform/linux-generic/odp_buffer.c
>>> index e54e0e7..2ee0461 100644
>>> --- a/platform/linux-generic/odp_buffer.c
>>> +++ b/platform/linux-generic/odp_buffer.c
>>> @@ -52,7 +52,7 @@ int odp_buffer_snprint(char *str, size_t n,
>>> odp_buffer_t buf)
>>>         int len = 0;
>>>         if (!odp_buffer_is_valid(buf)) {
>>> -               printf("Buffer is not valid.\n");
>>> +               ODP_PRI("Buffer is not valid.\n");
>>>                 return len;
>>>         }
>>>   @@ -98,7 +98,7 @@ void odp_buffer_print(odp_buffer_t buf)
>>>         len = odp_buffer_snprint(str, max_len-1, buf);
>>>         str[len] = 0;
>>>   -     printf("\n%s\n", str);
>>> +       ODP_PRI("\n%s\n", str);
>>>   }
>>>     void odp_buffer_copy_scatter(odp_buffer_t buf_dst, odp_buffer_t
>>> buf_src)
>>> diff --git a/platform/linux-generic/odp_buffer_pool.c
>>> b/platform/linux-generic/odp_buffer_pool.c
>>> index a48d7d6..3555166 100644
>>> --- a/platform/linux-generic/odp_buffer_pool.c
>>> +++ b/platform/linux-generic/odp_buffer_pool.c
>>> @@ -523,20 +523,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>>> pool_hdl)
>>>         pool_id = pool_handle_to_index(pool_hdl);
>>>         pool    = get_pool_entry(pool_id);
>>>   -     printf("Pool info\n");
>>> -       printf("---------\n");
>>> -       printf("  pool          %i\n",           pool->s.pool_hdl);
>>> -       printf("  name          %s\n",           pool->s.name);
>>> -       printf("  pool base     %p\n",           pool->s.pool_base_addr);
>>> -       printf("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
>>> -       printf("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
>>> -       printf("  buf size      %zu\n",          pool->s.user_size);
>>> -       printf("  buf align     %zu\n",          pool->s.user_align);
>>> -       printf("  hdr size      %zu\n",          pool->s.hdr_size);
>>> -       printf("  alloc size    %zu\n",          pool->s.buf_size);
>>> -       printf("  offset to hdr %zu\n",          pool->s.buf_offset);
>>> -       printf("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
>>> -       printf("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>>> +       ODP_PRI("Pool info\n");
>>> +       ODP_PRI("---------\n");
>>> +       ODP_PRI("  pool          %i\n",           pool->s.pool_hdl);
>>> +       ODP_PRI("  name          %s\n",           pool->s.name);
>>> +       ODP_PRI("  pool base     %p\n",
>>>  pool->s.pool_base_addr);
>>> +       ODP_PRI("  buf base      0x%"PRIxPTR"\n", pool->s.buf_base);
>>> +       ODP_PRI("  pool size     0x%"PRIx64"\n",  pool->s.pool_size);
>>> +       ODP_PRI("  buf size      %zu\n",          pool->s.user_size);
>>> +       ODP_PRI("  buf align     %zu\n",          pool->s.user_align);
>>> +       ODP_PRI("  hdr size      %zu\n",          pool->s.hdr_size);
>>> +       ODP_PRI("  alloc size    %zu\n",          pool->s.buf_size);
>>> +       ODP_PRI("  offset to hdr %zu\n",          pool->s.buf_offset);
>>> +       ODP_PRI("  num bufs      %"PRIu64"\n",    pool->s.num_bufs);
>>> +       ODP_PRI("  free bufs     %"PRIu64"\n",    pool->s.free_bufs);
>>>         /* first chunk */
>>>         chunk_hdr = pool->s.head;
>>> @@ -546,7 +546,7 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>>> pool_hdl)
>>>                 return;
>>>         }
>>>   -     printf("\n  First chunk\n");
>>> +       ODP_PRI("\n  First chunk\n");
>>>         for (i = 0; i < chunk_hdr->chunk.num_bufs - 1; i++) {
>>>                 uint32_t index;
>>> @@ -555,20 +555,20 @@ void odp_buffer_pool_print(odp_buffer_pool_t
>>> pool_hdl)
>>>                 index = chunk_hdr->chunk.buf_index[i];
>>>                 hdr   = index_to_hdr(pool, index);
>>>   -             printf("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr,
>>> index);
>>> +               ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i, hdr->addr,
>>> index);
>>>         }
>>>   -     printf("  [%i] addr %p, id %"PRIu32"\n", i,
>>> chunk_hdr->buf_hdr.addr,
>>> -              chunk_hdr->buf_hdr.index);
>>> +       ODP_PRI("  [%i] addr %p, id %"PRIu32"\n", i,
>>> chunk_hdr->buf_hdr.addr,
>>> +               chunk_hdr->buf_hdr.index);
>>>         /* next chunk */
>>>         chunk_hdr = next_chunk(pool, chunk_hdr);
>>>         if (chunk_hdr) {
>>> -               printf("  Next chunk\n");
>>> -               printf("  addr %p, id %"PRIu32"\n",
>>> chunk_hdr->buf_hdr.addr,
>>> -                      chunk_hdr->buf_hdr.index);
>>> +               ODP_PRI("  Next chunk\n");
>>> +               ODP_PRI("  addr %p, id %"PRIu32"\n",
>>> chunk_hdr->buf_hdr.addr,
>>> +                       chunk_hdr->buf_hdr.index);
>>>         }
>>>   -     printf("\n");
>>> +       ODP_PRI("\n");
>>>   }
>>> diff --git a/platform/linux-generic/odp_packet.c
>>> b/platform/linux-generic/odp_packet.c
>>> index 82ea879..7fe9ed0 100644
>>> --- a/platform/linux-generic/odp_packet.c
>>> +++ b/platform/linux-generic/odp_packet.c
>>> @@ -346,7 +346,7 @@ void odp_packet_print(odp_packet_t pkt)
>>>                         "  input        %u\n", hdr->input);
>>>         str[len] = '\0';
>>>   -     printf("\n%s\n", str);
>>> +       ODP_PRI("\n%s\n", str);
>>>   }
>>>     int odp_packet_copy(odp_packet_t pkt_dst, odp_packet_t pkt_src)
>>> diff --git a/platform/linux-generic/odp_shared_memory.c
>>> b/platform/linux-generic/odp_shared_memory.c
>>> index 24a5d60..cd4415d 100644
>>> --- a/platform/linux-generic/odp_shared_memory.c
>>> +++ b/platform/linux-generic/odp_shared_memory.c
>>> @@ -285,14 +285,14 @@ void odp_shm_print_all(void)
>>>   {
>>>         int i;
>>>   -     printf("\nShared memory\n");
>>> -       printf("--------------\n");
>>> -       printf("  page size:      %"PRIu64" kB\n", odp_sys_page_size() /
>>> 1024);
>>> -       printf("  huge page size: %"PRIu64" kB\n",
>>> -              odp_sys_huge_page_size() / 1024);
>>> -       printf("\n");
>>> +       ODP_PRI("\nShared memory\n");
>>> +       ODP_PRI("--------------\n");
>>> +       ODP_PRI("  page size:      %"PRIu64" kB\n", odp_sys_page_size()
>>> / 1024);
>>> +       ODP_PRI("  huge page size: %"PRIu64" kB\n",
>>> +               odp_sys_huge_page_size() / 1024);
>>> +       ODP_PRI("\n");
>>>   -     printf("  id name                       kB align huge addr\n");
>>> +       ODP_PRI("  id name                       kB align huge addr\n");
>>>         for (i = 0; i < ODP_SHM_NUM_BLOCKS; i++) {
>>>                 odp_shm_block_t *block;
>>> @@ -300,15 +300,15 @@ void odp_shm_print_all(void)
>>>                 block = &odp_shm_tbl->block[i];
>>>                 if (block->addr) {
>>> -                       printf("  %2i %-24s %4"PRIu64"  %4"PRIu64" %2c
>>>  %p\n",
>>> -                              i,
>>> -                              block->name,
>>> -                              block->size/1024,
>>> -                              block->align,
>>> -                              (block->huge ? '*' : ' '),
>>> -                              block->addr);
>>> +                       ODP_PRI("  %2i %-24s %4"PRIu64"  %4"PRIu64" %2c
>>>  %p\n",
>>> +                               i,
>>> +                               block->name,
>>> +                               block->size/1024,
>>> +                               block->align,
>>> +                               (block->huge ? '*' : ' '),
>>> +                               block->addr);
>>>                 }
>>>         }
>>>   -     printf("\n");
>>> +       ODP_PRI("\n");
>>>   }
>>>
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
>
> --
> *Mike Holmes*
> Linaro  Sr Technical Manager
> LNG - ODP
>

linux-generic: Replace printf with ODP_PRI

Commit Message

Comments

Patch