tracepoint: fix compilation with C++

  trace_mem is declared as 'void *' which triggers following error:
'...invalid conversion from ‘void*’ to ‘__rte_trace_header*’
[-fpermissive]...'

Fix this by changing void to struct __rte_trace_header
---
 lib/librte_eal/common/eal_common_trace.c | 2 +-
 lib/librte_eal/include/rte_trace_point.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
  

Hello Pawel,

Thanks for contributing to DPDK.


On Tue, Aug 4, 2020 at 7:52 PM Pawel Wodkowski <pawelwod@gmail.com> wrote:
>
> trace_mem is declared as 'void *' which triggers following error:
> '...invalid conversion from ‘void*’ to ‘__rte_trace_header*’
> [-fpermissive]...'
>
> Fix this by changing void to struct __rte_trace_header

I Cc'd trace maintainers.
For subsequent revisions, you can do directly Cc them by calling the
get-maintainer.sh script like this:
$ git send-email --to dev@dpdk.org --cc-cmd devtools/get-maintainer.sh $patch


On the form of the patch, this patch is missing a signed-off tag,
please have a look at the contribution guidelines:
https://doc.dpdk.org/guides/contributing/patches.html
Do not forget to chain replies using the --in-reply-to option for git
send-email.

This is a fix, so we expect a Fixes: tag in the commitlog.
This tag should point at the original commit that introduced the issue.


Looking at the CI report
http://mails.dpdk.org/archives/test-report/2020-August/147927.html,
you will see that the windows part is missing an update.

$ git grep -wl trace_mem
lib/librte_eal/common/eal_common_trace.c
lib/librte_eal/include/rte_trace_point.h
lib/librte_eal/windows/eal.c
  

On 05.08.2020 10:00, David Marchand wrote:
> Hello Pawel,
>
> Thanks for contributing to DPDK.
>
>
Thank for all tips. I will do that and resend V2.

Paweł
  

Hello Pawel, 

See answers inline.

Regards
Sunil Kumar Kori

>-----Original Message-----
>From: David Marchand <david.marchand@redhat.com>
>Sent: Wednesday, August 5, 2020 1:30 PM
>To: Pawel Wodkowski <pawelwod@gmail.com>
>Cc: dev <dev@dpdk.org>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>Sunil Kumar Kori <skori@marvell.com>
>Subject: [EXT] Re: [dpdk-dev] [PATCH] tracepoint: fix compilation with C++
>
>External Email
>
>----------------------------------------------------------------------
>Hello Pawel,
>
>Thanks for contributing to DPDK.
>
>
>On Tue, Aug 4, 2020 at 7:52 PM Pawel Wodkowski <pawelwod@gmail.com>
>wrote:
>>
>> trace_mem is declared as 'void *' which triggers following error:
>> '...invalid conversion from ‘void*’ to ‘__rte_trace_header*’
>> [-fpermissive]...'
>>
>> Fix this by changing void to struct __rte_trace_header

trace_mem is intentionally kept as void * so that it can not be accessed by application directly as it part of global header.
If I understood the problem correctly, it is because of using trace_mem without typecasting and GCC does not report it as error
due to implicit typecast and G++ reports it as error as it does not do implicit typecasting. 

If this is the case then, I think it is better to typecast the trace_mem where ever it is being used. Anyways that will be safe for both GCC and G++.
@Jerin Jacob Kollanukkaran Please suggest. If you have some thing mind. 

[snippet]
--
  

On 06.08.2020 08:44, Sunil Kumar Kori wrote:
> Hello Pawel,
>
> See answers inline.
>
> Regards
> Sunil Kumar Kori
>
>> -----Original Message-----
>> From: David Marchand <david.marchand@redhat.com>
>> Sent: Wednesday, August 5, 2020 1:30 PM
>> To: Pawel Wodkowski <pawelwod@gmail.com>
>> Cc: dev <dev@dpdk.org>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>> Sunil Kumar Kori <skori@marvell.com>
>> Subject: [EXT] Re: [dpdk-dev] [PATCH] tracepoint: fix compilation with C++
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> Hello Pawel,
>>
>> Thanks for contributing to DPDK.
>>
>>
>> On Tue, Aug 4, 2020 at 7:52 PM Pawel Wodkowski <pawelwod@gmail.com>
>> wrote:
>>> trace_mem is declared as 'void *' which triggers following error:
>>> '...invalid conversion from ‘void*’ to ‘__rte_trace_header*’
>>> [-fpermissive]...'
>>>
>>> Fix this by changing void to struct __rte_trace_header
> trace_mem is intentionally kept as void * so that it can not be accessed by application directly as it part of global header.
But this structure is well defined in this file anyway. It can be casted 
to 'struct __rte_trace_header *' and used.
Isn't the double underscore prefix good enough warning that it is 
internal datatype?

But anyway, I'm not here to discuss the architecture of DPDK so lets go 
to the next point.

> If I understood the problem correctly, it is because of using trace_mem without typecasting and GCC does not report it as error
> due to implicit typecast and G++ reports it as error as it does not do implicit typecasting.
Actually it is because in C++ implicit cast from void pointer is not 
allowed. So when it is used like this

      struct __rte_trace_header *trace = RTE_PER_LCORE(trace_mem);

it triggers this type of error:

include/rte_trace_point.h: In function ‘void* 
__rte_trace_mem_get(uint64_t)’:
include/rte_per_lcore.h:44:46: error: invalid conversion from ‘void*’ to 
‘__rte_trace_header*’ [-fpermissive]
  #define RTE_PER_LCORE(name) (per_lcore_##name)
                                         ^
nclude/rte_trace_point.h:303:37: note: in expansion of macro ‘RTE_PER_LCORE’
   struct __rte_trace_header *trace = RTE_PER_LCORE(trace_mem);
                                                           ^

One can add '-fpermisive' to allow this type of casting but it is only a 
workaround in C++ code. As you mentioned,
other solution is typecast to __rte_trace_header but this is not needed 
in C and it have "__" prefix this why I decided
to change 'void *' into 'struct __rte_trace_header *'.

>   
>
> If this is the case then, I think it is better to typecast the trace_mem where ever it is being used. Anyways that will be safe for both GCC and G++.
> @Jerin Jacob Kollanukkaran Please suggest. If you have some thing mind.

I'm fine any solution that make this code compile with C++. Please let 
me know what is the decision then I can make V2.

Paweł
>   
>
> [snippet]
> --
>
  

Answers inline.

Regards
Sunil Kumar Kori

>-----Original Message-----
>From: Pawel Wodkowski <pawelwod@gmail.com>
>Sent: Friday, August 7, 2020 12:14 AM
>To: Sunil Kumar Kori <skori@marvell.com>; Jerin Jacob Kollanukkaran
><jerinj@marvell.com>
>Cc: dev <dev@dpdk.org>; David Marchand <david.marchand@redhat.com>
>Subject: Re: [EXT] Re: [dpdk-dev] [PATCH] tracepoint: fix compilation with C++
>
>On 06.08.2020 08:44, Sunil Kumar Kori wrote:
>> Hello Pawel,
>>
>> See answers inline.
>>
>> Regards
>> Sunil Kumar Kori
>>
>>> -----Original Message-----
>>> From: David Marchand <david.marchand@redhat.com>
>>> Sent: Wednesday, August 5, 2020 1:30 PM
>>> To: Pawel Wodkowski <pawelwod@gmail.com>
>>> Cc: dev <dev@dpdk.org>; Jerin Jacob Kollanukkaran
>>> <jerinj@marvell.com>; Sunil Kumar Kori <skori@marvell.com>
>>> Subject: [EXT] Re: [dpdk-dev] [PATCH] tracepoint: fix compilation
>>> with C++
>>>
>>> External Email
>>>
>>> ---------------------------------------------------------------------
>>> -
>>> Hello Pawel,
>>>
>>> Thanks for contributing to DPDK.
>>>
>>>
>>> On Tue, Aug 4, 2020 at 7:52 PM Pawel Wodkowski
><pawelwod@gmail.com>
>>> wrote:
>>>> trace_mem is declared as 'void *' which triggers following error:
>>>> '...invalid conversion from ‘void*’ to ‘__rte_trace_header*’
>>>> [-fpermissive]...'
>>>>
>>>> Fix this by changing void to struct __rte_trace_header
>> trace_mem is intentionally kept as void * so that it can not be accessed by
>application directly as it part of global header.
>But this structure is well defined in this file anyway. It can be casted to 'struct
>__rte_trace_header *' and used.
>Isn't the double underscore prefix good enough warning that it is internal
>datatype?
>
>But anyway, I'm not here to discuss the architecture of DPDK so lets go to the
>next point.
>
>> If I understood the problem correctly, it is because of using
>> trace_mem without typecasting and GCC does not report it as error due to
>implicit typecast and G++ reports it as error as it does not do implicit
>typecasting.
>Actually it is because in C++ implicit cast from void pointer is not allowed. So
>when it is used like this
>
>      struct __rte_trace_header *trace = RTE_PER_LCORE(trace_mem);
>
>it triggers this type of error:
>
>include/rte_trace_point.h: In function ‘void*
>__rte_trace_mem_get(uint64_t)’:
>include/rte_per_lcore.h:44:46: error: invalid conversion from ‘void*’ to
>‘__rte_trace_header*’ [-fpermissive]
>  #define RTE_PER_LCORE(name) (per_lcore_##name)
>                                         ^
>nclude/rte_trace_point.h:303:37: note: in expansion of macro
>‘RTE_PER_LCORE’
>   struct __rte_trace_header *trace = RTE_PER_LCORE(trace_mem);
>                                                           ^
>
>One can add '-fpermisive' to allow this type of casting but it is only a
>workaround in C++ code. As you mentioned, other solution is typecast to
>__rte_trace_header but this is not needed in C and it have "__" prefix this why
>I decided to change 'void *' into 'struct __rte_trace_header *'.
>

But changing 'void *' to 'struct __rte_trace_header *' will change the interpretation of trace_mem from user perspective which is against the design.
I would request you to go for the proposed solution. Although  there is no need of this for GCC but if there is no harm. 

Also consider a use case where system is designed like  'structure is exposed to application as opaque pointer(void *) and typecasted by the underlying implementation before using it'.
Then changing opaque pointer to structure type is not expected.

>>
>>
>> If this is the case then, I think it is better to typecast the trace_mem where
>ever it is being used. Anyways that will be safe for both GCC and G++.
>> @Jerin Jacob Kollanukkaran Please suggest. If you have some thing mind.
>
>I'm fine any solution that make this code compile with C++. Please let me
>know what is the decision then I can make V2.
>
>Paweł
>>
>>
>> [snippet]
>> --
>>