Re: [dpdk-dev] [PATCH v3 07/12] log: add log stream accessor

[Kevin Traynor <ktraynor@redhat.com>]

On 25/10/2019 14:56, David Marchand wrote:
> Define an accessor so that users can write their debug message to the
> same stream than the rte_log infrastructure.
> Use it in the qat infrastructure.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> Changelog since v1:
> - use ternary operator,
> 
> ---
>  drivers/common/qat/qat_logs.c           |  3 +--
>  drivers/common/qat/qat_logs.h           |  3 +--
>  lib/librte_eal/common/eal_common_log.c  | 33 +++++++++++++++++++--------------
>  lib/librte_eal/common/include/rte_log.h | 13 +++++++++++++
>  lib/librte_eal/rte_eal_version.map      |  3 +++
>  5 files changed, 37 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/common/qat/qat_logs.c b/drivers/common/qat/qat_logs.c
> index 7a86170..f97aba1 100644
> --- a/drivers/common/qat/qat_logs.c
> +++ b/drivers/common/qat/qat_logs.c
> @@ -19,8 +19,7 @@ qat_hexdump_log(uint32_t level, uint32_t logtype, const char *title,
>  	if (level > (uint32_t)(rte_log_get_level(logtype)))
>  		return 0;
>  
> -	rte_hexdump(rte_logs.file == NULL ? stderr : rte_logs.file,
> -				title, buf, len);
> +	rte_hexdump(rte_log_get_stream(), title, buf, len);
>  	return 0;
>  }
>  
> diff --git a/drivers/common/qat/qat_logs.h b/drivers/common/qat/qat_logs.h
> index 4baea12..2e4d394 100644
> --- a/drivers/common/qat/qat_logs.h
> +++ b/drivers/common/qat/qat_logs.h
> @@ -24,8 +24,7 @@ extern int qat_dp_logtype;
>   *
>   * Dump out the message buffer in a special hex dump output format with
>   * characters printed for each line of 16 hex values. The message will be sent
> - * to the stream defined by rte_logs.file or to stderr in case of rte_logs.file
> - * is undefined.
> + * to the stream used by the rte_log infrastructure.
>   */
>  int
>  qat_hexdump_log(uint32_t level, uint32_t logtype, const char *title,
> diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> index 4f6f227..e0a7bef 100644
> --- a/lib/librte_eal/common/eal_common_log.c
> +++ b/lib/librte_eal/common/eal_common_log.c
> @@ -71,6 +71,24 @@ rte_openlog_stream(FILE *f)
>  	return 0;
>  }
>  
> +FILE *
> +rte_log_get_stream(void)
> +{
> +	FILE *f = rte_logs.file;
> +
> +	if (f == NULL) {
> +		/*
> +		 * Grab the current value of stderr here, rather than
> +		 * just initializing default_log_stream to stderr. This
> +		 * ensures that we will always use the current value
> +		 * of stderr, even if the application closes and
> +		 * reopens it.
> +		 */
> +		return default_log_stream ? : stderr;
> +	}
> +	return f;
> +}
> +
>  /* Set global log level */
>  void
>  rte_log_set_global_level(uint32_t level)
> @@ -396,21 +414,8 @@ rte_log_dump(FILE *f)
>  int
>  rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
>  {
> +	FILE *f = rte_log_get_stream();
>  	int ret;
> -	FILE *f = rte_logs.file;
> -	if (f == NULL) {
> -		f = default_log_stream;
> -		if (f == NULL) {
> -			/*
> -			 * Grab the current value of stderr here, rather than
> -			 * just initializing default_log_stream to stderr. This
> -			 * ensures that we will always use the current value
> -			 * of stderr, even if the application closes and
> -			 * reopens it.
> -			 */
> -			f = stderr;
> -		}
> -	}
>  
>  	if (level > rte_logs.level)
>  		return 0;
> diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
> index cbb4184..1bb0e66 100644
> --- a/lib/librte_eal/common/include/rte_log.h
> +++ b/lib/librte_eal/common/include/rte_log.h
> @@ -102,6 +102,19 @@ extern struct rte_logs rte_logs;
>  int rte_openlog_stream(FILE *f);
>  
>  /**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Retrieve the stream used by the logging system (see rte_openlog_stream()
> + * to change it).
> + *
> + * @return
> + *   Pointer to the stream.
> + */
> +__rte_experimental
> +FILE *rte_log_get_stream(void);
> +
> +/**
>   * Set the global log level.
>   *
>   * After this call, logs with a level lower or equal than the level
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index 0887549..6d7e0e4 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -417,4 +417,7 @@ EXPERIMENTAL {
>  	rte_mcfg_timer_lock;
>  	rte_mcfg_timer_unlock;
>  	rte_rand_max;
> +
> +	# added in 19.11
> +	rte_log_get_stream;
>  };
> 

Acked-by: Kevin Traynor <ktraynor@redhat.com>