ODP_SAY()?

On Tue, Nov 18, 2014 at 1:22 PM, Mike Holmes <[email protected]> wrote:

>
>
> On 18 November 2014 14:04, Bill Fischofer <[email protected]>
> 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 <[email protected]>
>> wrote:
>>
>>>
>>>
>>> On 18 November 2014 13:50, Bill Fischofer <[email protected]>
>>> 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 <[email protected]>
>>>> 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 <[email protected]>
>>>>> ---
>>>>>
>>>>> 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
>>>>> [email protected]
>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> *Mike Holmes*
>>> Linaro  Sr Technical Manager
>>> LNG - ODP
>>>
>>
>>
>
>
> --
> *Mike Holmes*
> Linaro  Sr Technical Manager
> LNG - ODP
>
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to