Hi Pawel, On 02/24/2015 12:12 PM, Wodkowski, PawelX wrote: > > >> -----Original Message----- >> From: Olivier MATZ [mailto:olivier.matz at 6wind.com] >> Sent: Tuesday, February 24, 2015 11:39 AM >> To: Wodkowski, PawelX; dev at 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