[{"id":2226,"web_url":"https://patchwork.libcamera.org/comment/2226/","msgid":"<20190711181715.GP5247@pendragon.ideasonboard.com>","date":"2019-07-11T18:17:15","subject":"Re: [libcamera-devel] [PATCH v3 1/2] libcamera: logging: add\n\tlogging API for applications","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Fri, Jul 12, 2019 at 03:05:24AM +0900, Paul Elder wrote:\n> Currently the log file and the log level can only be set via environment\n> variables, but applications may also want to set the log file and the\n> log level at run time. Provide an API for this.\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> ---\n> Changes in v3:\n> - make setLogFile revert to stderr on empty arg\n> - make both setLogFile and setLogLevel ignore empty args (not check for\n>   error)\n> - better docs for setLogFile\n> \n> Changes in v2:\n> - add documentation\n> - actually set the log level\n> \n>  include/libcamera/logging.h   | 17 ++++++++++\n>  include/libcamera/meson.build |  1 +\n>  src/libcamera/log.cpp         | 60 +++++++++++++++++++++++++++++++++++\n>  3 files changed, 78 insertions(+)\n>  create mode 100644 include/libcamera/logging.h\n> \n> diff --git a/include/libcamera/logging.h b/include/libcamera/logging.h\n> new file mode 100644\n> index 0000000..c8a048e\n> --- /dev/null\n> +++ b/include/libcamera/logging.h\n> @@ -0,0 +1,17 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * logging.h - Logging infrastructure\n> + */\n> +#ifndef __LIBCAMERA_LOGGING_H__\n> +#define __LIBCAMERA_LOGGING_H__\n> +\n> +namespace libcamera {\n> +\n> +void logSetFile(const char *file);\n> +void logSetLevel(const char *category, const char *level);\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_LOGGING_H__ */\n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 972513f..920eb5f 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -9,6 +9,7 @@ libcamera_api = files([\n>      'geometry.h',\n>      'ipa/ipa_interface.h',\n>      'ipa/ipa_module_info.h',\n> +    'logging.h',\n>      'object.h',\n>      'request.h',\n>      'signal.h',\n> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n> index 0ba276e..11bac80 100644\n> --- a/src/libcamera/log.cpp\n> +++ b/src/libcamera/log.cpp\n> @@ -69,6 +69,9 @@ private:\n>  \tvoid parseLogLevels();\n>  \tstatic LogSeverity parseLogLevel(const std::string &level);\n>  \n> +\tfriend void logSetFile(const char *file);\n> +\tfriend void logSetLevel(const char *category, const char *level);\n> +\n>  \tfriend LogCategory;\n>  \tvoid registerCategory(LogCategory *category);\n>  \tvoid unregisterCategory(LogCategory *category);\n> @@ -80,6 +83,63 @@ private:\n>  \tstd::ostream *output_;\n>  };\n>  \n> +/**\n> + * \\brief Set the log file\n> + * \\param[in] file Log file\n\n\"Full path to the log file\"\n\n> + *\n> + * This functions sets the logging output file to \\a file.\n> + * If \\a file is an empty string, then the output file will be set to stderr.\n\n * This function sets the logging output file to \\a file. The previous log file,\n * if any, is closed, and all new log messages will be written to the new log\n * file.\n *\n * If \\a file is a null pointer, the log is directed to stderr. If the\n * function returns an error, the log file is not changed.\n *\n * \\return Zero on success, or a negative error code otherwise.\n\n> + */\n> +void logSetFile(const char *file)\n> +{\n> +\tLogger *logger = Logger::instance();\n> +\n> +\tif (!file[0]) {\n\n\tif (!file) {\n\n(I was tired when reviewing the previous version...)\n\n> +\t\tlogger->file_.close();\n> +\t\tlogger->output_ = &std::cerr;\n\nSwap those two lines to avoid races (we'll have to implement locking,\nbut that's for later).\n\n> +\t\treturn;\n> +\t}\n> +\n> +\tstd::ofstream logFile(file);\n> +\tif (!logFile.good())\n> +\t\treturn;\n\n\t\treturn -EINVAL;\n\n> +\n> +\tif (logger->output_ != &std::cerr)\n> +\t\tlogger->file_.close();\n> +\tlogger->file_ = std::move(logFile);\n> +\tlogger->output_ = &logger->file_;\n> +}\n> +\n> +/**\n> + * \\brief Set the log level\n> + * \\param[in] category Logging category\n> + * \\param[in] level Log level\n> + *\n> + * This function sets the log level of \\a category to \\a level.\n> + * \\a level shall be one of the following strings:\n> + * - \"DEBUG\"\n> + * - \"INFO\"\n> + * - \"WARN\"\n> + * - \"ERROR\"\n> + * - \"FATAL\"\n> + *\n> + * \"*\" is not a valid \\a category for this function.\n> + */\n> +void logSetLevel(const char *category, const char *level)\n> +{\n> +\tLogger *logger = Logger::instance();\n> +\tstd::string cat(category);\n\nThis is unused.\n\n> +\tstd::string lev(level);\n\nThis is used in a single location, you can write\nLogger::parseLogLevel(level) and remove lev.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> +\n> +\tLogSeverity severity = Logger::parseLogLevel(lev);\n> +\tif (severity == LogInvalid)\n> +\t\treturn;\n> +\n> +\tfor (LogCategory *c : logger->categories_)\n> +\t\tif (!strcmp(c->name(), category))\n> +\t\t\tc->setSeverity(severity);\n> +}\n> +\n>  /**\n>   * \\brief Retrieve the logger instance\n>   *","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7142260BC8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 11 Jul 2019 20:17:43 +0200 (CEST)","from pendragon.ideasonboard.com (softbank126163157105.bbtec.net\n\t[126.163.157.105])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2550831C;\n\tThu, 11 Jul 2019 20:17:41 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1562869063;\n\tbh=sWwgHt9hoXXZtmYred7n21Qv7BVI3BUpMBQjBEDmoQ4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=B3PW5+Kr68tp2zGco2NhRTVdBMZy5QeKii6C/Zri9o3kr+kHDDcn7HoOvDR53u/FG\n\tgkSQBWS/nuCb8bA5HRTDhYmw8mdp3HVzKPGf6edQB8YRACo1rMu9ExBRWDd13prl2D\n\tjBM98GLJ4cnDkxxq8I50/kGreezcsSFM09+OPZ4g=","Date":"Thu, 11 Jul 2019 21:17:15 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190711181715.GP5247@pendragon.ideasonboard.com>","References":"<20190711180525.31519-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190711180525.31519-1-paul.elder@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] libcamera: logging: add\n\tlogging API for applications","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 11 Jul 2019 18:17:43 -0000"}}]