[libcamera-devel,v3,1/2] libcamera: logging: add logging API for applications

1657 diff mbox series

Currently the log file and the log level can only be set via environment
variables, but applications may also want to set the log file and the
log level at run time. Provide an API for this.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
Changes in v3:
- make setLogFile revert to stderr on empty arg
- make both setLogFile and setLogLevel ignore empty args (not check for
  error)
- better docs for setLogFile

Changes in v2:
- add documentation
- actually set the log level

 include/libcamera/logging.h   | 17 ++++++++++
 include/libcamera/meson.build |  1 +
 src/libcamera/log.cpp         | 60 +++++++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+)
 create mode 100644 include/libcamera/logging.h

Hi Paul,

Thank you for the patch.

On Fri, Jul 12, 2019 at 03:05:24AM +0900, Paul Elder wrote:
> Currently the log file and the log level can only be set via environment
> variables, but applications may also want to set the log file and the
> log level at run time. Provide an API for this.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> Changes in v3:
> - make setLogFile revert to stderr on empty arg
> - make both setLogFile and setLogLevel ignore empty args (not check for
>   error)
> - better docs for setLogFile
> 
> Changes in v2:
> - add documentation
> - actually set the log level
> 
>  include/libcamera/logging.h   | 17 ++++++++++
>  include/libcamera/meson.build |  1 +
>  src/libcamera/log.cpp         | 60 +++++++++++++++++++++++++++++++++++
>  3 files changed, 78 insertions(+)
>  create mode 100644 include/libcamera/logging.h
> 
> diff --git a/include/libcamera/logging.h b/include/libcamera/logging.h
> new file mode 100644
> index 0000000..c8a048e
> --- /dev/null
> +++ b/include/libcamera/logging.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * logging.h - Logging infrastructure
> + */
> +#ifndef __LIBCAMERA_LOGGING_H__
> +#define __LIBCAMERA_LOGGING_H__
> +
> +namespace libcamera {
> +
> +void logSetFile(const char *file);
> +void logSetLevel(const char *category, const char *level);
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_LOGGING_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 972513f..920eb5f 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -9,6 +9,7 @@ libcamera_api = files([
>      'geometry.h',
>      'ipa/ipa_interface.h',
>      'ipa/ipa_module_info.h',
> +    'logging.h',
>      'object.h',
>      'request.h',
>      'signal.h',
> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp
> index 0ba276e..11bac80 100644
> --- a/src/libcamera/log.cpp
> +++ b/src/libcamera/log.cpp
> @@ -69,6 +69,9 @@ private:
>  	void parseLogLevels();
>  	static LogSeverity parseLogLevel(const std::string &level);
>  
> +	friend void logSetFile(const char *file);
> +	friend void logSetLevel(const char *category, const char *level);
> +
>  	friend LogCategory;
>  	void registerCategory(LogCategory *category);
>  	void unregisterCategory(LogCategory *category);
> @@ -80,6 +83,63 @@ private:
>  	std::ostream *output_;
>  };
>  
> +/**
> + * \brief Set the log file
> + * \param[in] file Log file

"Full path to the log file"

> + *
> + * This functions sets the logging output file to \a file.
> + * If \a file is an empty string, then the output file will be set to stderr.

 * This function sets the logging output file to \a file. The previous log file,
 * if any, is closed, and all new log messages will be written to the new log
 * file.
 *
 * If \a file is a null pointer, the log is directed to stderr. If the
 * function returns an error, the log file is not changed.
 *
 * \return Zero on success, or a negative error code otherwise.

> + */
> +void logSetFile(const char *file)
> +{
> +	Logger *logger = Logger::instance();
> +
> +	if (!file[0]) {

	if (!file) {

(I was tired when reviewing the previous version...)

> +		logger->file_.close();
> +		logger->output_ = &std::cerr;

Swap those two lines to avoid races (we'll have to implement locking,
but that's for later).

> +		return;
> +	}
> +
> +	std::ofstream logFile(file);
> +	if (!logFile.good())
> +		return;

		return -EINVAL;

> +
> +	if (logger->output_ != &std::cerr)
> +		logger->file_.close();
> +	logger->file_ = std::move(logFile);
> +	logger->output_ = &logger->file_;
> +}
> +
> +/**
> + * \brief Set the log level
> + * \param[in] category Logging category
> + * \param[in] level Log level
> + *
> + * This function sets the log level of \a category to \a level.
> + * \a level shall be one of the following strings:
> + * - "DEBUG"
> + * - "INFO"
> + * - "WARN"
> + * - "ERROR"
> + * - "FATAL"
> + *
> + * "*" is not a valid \a category for this function.
> + */
> +void logSetLevel(const char *category, const char *level)
> +{
> +	Logger *logger = Logger::instance();
> +	std::string cat(category);

This is unused.

> +	std::string lev(level);

This is used in a single location, you can write
Logger::parseLogLevel(level) and remove lev.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +	LogSeverity severity = Logger::parseLogLevel(lev);
> +	if (severity == LogInvalid)
> +		return;
> +
> +	for (LogCategory *c : logger->categories_)
> +		if (!strcmp(c->name(), category))
> +			c->setSeverity(severity);
> +}
> +
>  /**
>   * \brief Retrieve the logger instance
>   *