Re: [dpdk-dev] [PATCH 1/5] rte_timer: fix invalid declaration of rte_timer_cb_t

[Olivier MATZ <olivier.matz@6wind.com>]

Hi Pawel,

On 02/24/2015 12:12 PM, Wodkowski, PawelX wrote:
>
>
>> -----Original Message-----
>> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
>> Sent: Tuesday, February 24, 2015 11:39 AM
>> To: Wodkowski, PawelX; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 1/5] rte_timer: fix invalid declaration of
>> rte_timer_cb_t
>>
>> Hi Pawel,
>>
>> On 02/23/2015 03:09 PM, Pawel Wodkowski wrote:
>>> Declaration for function pointer should be
>>> typedef ret_type (*type_name)(args...)
>>> not
>>> typedef ret_type (type_name)(args...)
>>>
>>> although compiler treat both of them the same, the static analysis tool
>>> like klocwork complain about that.
>>
>> Can you give some details about the reason why klocwork is
>> complaining?
>>
>> Looking at the C11 standard, it seems that this syntax is
>> legal. Please see EXAMPLE 4, page 156 of:
>> http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1570.pdf
>>
>
> Legal, might be. Problem is in using it. In struct rte_timer field 'f' if
> declared as pointer to rte_timer_cb_t but  __rte_timer_reset() expects
> rte_timer_cb_t. This have a little impact to real code but it is inconsistent
> with declaration, definition and rest of the library where first syntax is used.
> There are some places where second declaration is used but its usage there
> is consistent with declaration.
>
> I looked at the code in rest of library and for consistency I changed
> typedef rather than function declaration.

OK, got it.
The problem was not the declaration itself but the inconsistency
between the function arguments and the rte_timer structure.

If you plan to do a v2 (maybe for patch 5/5), do you think you can
reword the commit log and title to clarify this a bit more?

Thanks,
Olivier