Re: [dpdk-dev] [PATCH v9 02/10] eal: move OS common options functions

[Thomas Monjalon <thomas@monjalon.net>]

24/06/2020 10:28, talshn@mellanox.com:
> From: Tal Shnaiderman <talshn@mellanox.com>
> 
> Move common functions between Unix and Windows to eal_common_options.c.
> 
> Those functions are getter functions for rte_application_usage_hook.
[...]
> +/* Return a pointer to rte_usage_hook_t */
> +rte_usage_hook_t *

Why not returning rte_usage_hook_t directly?

> +eal_get_application_usage_hook(void)
> +{
> +	return &rte_application_usage_hook;
> +}

[...]
> +/* Set a per-application usage message */
> +rte_usage_hook_t
> +rte_set_application_usage_hook(rte_usage_hook_t usage_func)
> +{
> +	rte_usage_hook_t	old_func;

A single space is enough to declare the variable.

> +
> +	/* Will be NULL on the first call to denote the last usage routine. */
> +	old_func = rte_application_usage_hook;
> +	rte_application_usage_hook = usage_func;
> +
> +	return old_func;
> +}

[...]
>  eal_usage(const char *prgname)
>  {
> +	rte_usage_hook_t *hook = eal_get_application_usage_hook();
> +
>  	printf("\nUsage: %s ", prgname);
>  	eal_common_usage();
>  	/* Allow the application to print its usage message too if hook is set */
> -	if ( rte_application_usage_hook ) {
> +	if (*hook) {

Explicit test is better: if (*hook != NULL)
It could even be if (hook != NULL && *hook != NULL),
which asks the question why returning the pointer of the pointer?

>  		printf("===== Application Usage =====\n\n");
> -		rte_application_usage_hook(prgname);
> +		(*hook)(prgname);
>  	}
>  }