Re: [dpdk-dev] [EXT] Re: [PATCH] tracepoint: fix compilation with C++

[Pawel Wodkowski <pawelwod@gmail.com>]

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]
> --
>