Re: [dpdk-dev] [PATCH 02/11] log: define logtype register wrapper for drivers

[Andrew Rybchenko <arybchenko@solarflare.com>]

On September 4, 2019 22:42:12 Thomas Monjalon <thomas@monjalon.net> wrote:

> 04/09/2019 21:21, Andrew Rybchenko:
>> On 9/4/19 8:45 PM, Thomas Monjalon wrote:
>>> 03/09/2019 10:47, Ferruh Yigit:
>>>> On 9/3/2019 9:06 AM, David Marchand wrote:
>>>>> On Mon, Sep 2, 2019 at 4:29 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>>>> On 8/19/2019 12:41 PM, David Marchand wrote:
>>>>>>> The function rte_log_register_type_and_pick_level() fills a gap for
>>>>>>> dynamically loaded code (especially drivers) who would not pick up
>>>>>>> the log level passed at startup.
>>>>>>>
>>>>>>>
>>>>>>> Let's promote it to stable and export it for use by drivers via
>>>>>>> a wrapper.
>>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>>>>>> ---
>>> [...]
>>>>>>> /**
>>>>>>> - * @warning
>>>>>>> - * @b EXPERIMENTAL: this API may change without prior notice
>>>>>>> - *
>>>>>>> * Register a dynamic log type and try to pick its level from EAL options
>>> [...]
>>>>>>> -__rte_experimental
>>>>>>> int rte_log_register_type_and_pick_level(const char *name, uint32_t level_def);
>>>>>> +1 to remove experimental from the API.
>>> I am not sure about this function API.
>>> Why we combined register and level setting in one function?
>>
>> See [1]
>>
>> [1] http://git.dpdk.org/dpdk/commit/?id=b22e77c026
>
> Sorry, it does not explain why we mix both operations in one function.

Exactly to have one function instead of repeating two function calls 
everywhere. Log level should be set when log type is registered. Yes, it is 
possible to factor out a function just to pick log level, but I'm not sure 
it makes sense separately.

In fact may be it makes sense to substitute just register with this one 
(I.e. remove simple register from piblic API and do not highlight that 
level is picked up in function name).