[ccan] [PATCH] pr_log: a new module that provides a simple run-time controlled logging interface

Cody P Schafer dev at codyps.com
Mon Aug 17 14:24:36 AEST 2015


On Sun, Aug 16, 2015 at 9:52 PM, Rusty Russell <rusty at rustcorp.com.au> wrote:
> Cody P Schafer <dev at codyps.com> writes:
>> A simple printf logging infra where levels are detemined by the
>> value of the "DEBUG" environment variable.
>>
>> This is loosely based on the interfaces & functionality of Linux's
>> printk() and pr_*() wrapper macros.
>>
>> Note that the current implimentation uses "<N>" prefixes (where N is a
>> syslog level in ascii), allowing other programs that parse log output
>> (like systemd's journald) to know what the priority level is.
>
> I like the idea, but I can't believe you're seriously proposing
> imitating the kernel interface.  You know they're deprecating printk()
> without the level arg, right?

I wasn't aware (I don't follow kernel devel closely at the moment),
but I don't really expect most people to use the pr_log() function
directly. If they want something without a debug level, printf() is
readily avaliable.

> I mean, it's OK if you want to add <N> to the output, but it's weird
> from an API POV.

So I considered a few different ways to go about this, initially
considering a log(int, char *, ...) function (which has the
unfortunate behavior of bloating the code size compared to a single
format string), then switched to a custom-string prefix mechanism, and
then decided (because it played nice with systemd) that I didn't
really have a compelling reason not to just use the kernel-style <N>
prefixed format strings.

So I suppose I don't think it's all that funny. And, without googling
things, I expect the reason printk() is having support for non-level
args removed is to prevent people from adding more kernel messages
without considering at all what level is appropriate. I'm not
convinced a that would even make sense as a concern here as most
people are going to reach for printf() in those cases.

>
> Cheers,
> Rusty.
> PS.  You don't have to change it: the rule is that I will accept any
>      well-formed module.  I was just a bit surprised...
>
>> Signed-off-by: Cody P Schafer <dev at codyps.com>
>> ---
>>  Makefile-ccan          |  1 +
>>  ccan/pr_log/LICENSE    |  1 +
>>  ccan/pr_log/_info      | 41 +++++++++++++++++++++++++++++++++
>>  ccan/pr_log/pr_log.c   | 48 +++++++++++++++++++++++++++++++++++++++
>>  ccan/pr_log/pr_log.h   | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  ccan/pr_log/test/run.c | 24 ++++++++++++++++++++
>>  6 files changed, 176 insertions(+)
>>  create mode 120000 ccan/pr_log/LICENSE
>>  create mode 100644 ccan/pr_log/_info
>>  create mode 100644 ccan/pr_log/pr_log.c
>>  create mode 100644 ccan/pr_log/pr_log.h
>>  create mode 100644 ccan/pr_log/test/run.c
>>
>> diff --git a/Makefile-ccan b/Makefile-ccan
>> index 99c4910..8d13a4f 100644
>> --- a/Makefile-ccan
>> +++ b/Makefile-ccan
>> @@ -88,6 +88,7 @@ MODS_WITH_SRC := aga \
>>       ogg_to_pcm \
>>       opt \
>>       order \
>> +     pr_log \
>>       ptrint \
>>       ptr_valid \
>>       pushpull \
>> diff --git a/ccan/pr_log/LICENSE b/ccan/pr_log/LICENSE
>> new file mode 120000
>> index 0000000..dc314ec
>> --- /dev/null
>> +++ b/ccan/pr_log/LICENSE
>> @@ -0,0 +1 @@
>> +../../licenses/LGPL-2.1
>> \ No newline at end of file
>> diff --git a/ccan/pr_log/_info b/ccan/pr_log/_info
>> new file mode 100644
>> index 0000000..021933f
>> --- /dev/null
>> +++ b/ccan/pr_log/_info
>> @@ -0,0 +1,41 @@
>> +#include <string.h>
>> +#include "config.h"
>> +
>> +/**
>> + * pr_log - print things with varying levels of importance
>> + *
>> + * pr_log is a "logger" styled similarly to Linux's printk() and pr_*() macros.
>> + * The amount of debug output is controlled by the value of the `DEBUG`
>> + * environment variable.
>> + *
>> + * It provides work-alikes for Linux's pr_devel, pr_debug, pr_info, etc macros.
>> + *
>> + * Example:
>> + *   #include <ccan/pr_log/pr_log.h>
>> + *
>> + *   int main(int argc, char *argv[])
>> + *   {
>> + *           pr_debug("It's working\n");
>> + *           pr_log("I: Informational %d\n", 3);
>> + *           pr_log("-9: Really important\n");
>> + *           pr_log("9: Really un-important\n");
>> + *           return 0;
>> + *   }
>> + *
>> + * License: LGPL (v2.1 or any later version)
>> + * Author: Cody P Schafer <dev at codyps.com>
>> + */
>> +int main(int argc, char *argv[])
>> +{
>> +     /* Expect exactly one argument */
>> +     if (argc != 2)
>> +             return 1;
>> +
>> +     if (strcmp(argv[1], "depends") == 0) {
>> +             printf("ccan/compiler\n");
>> +             printf("ccan/str\n");
>> +             return 0;
>> +     }
>> +
>> +     return 1;
>> +}
>> diff --git a/ccan/pr_log/pr_log.c b/ccan/pr_log/pr_log.c
>> new file mode 100644
>> index 0000000..dc2b890
>> --- /dev/null
>> +++ b/ccan/pr_log/pr_log.c
>> @@ -0,0 +1,48 @@
>> +/* Licensed under LGPLv2.1+ - see LICENSE file for details */
>> +#include "pr_log.h"
>> +
>> +#include <ctype.h>
>> +#include <stdarg.h>
>> +#include <stdbool.h>
>> +#include <stdlib.h>
>> +#include <stdio.h>
>> +#include <limits.h>
>> +
>> +#include <ccan/str/str.h>
>> +
>> +#define DEBUG_NEED_INIT INT_MIN
>> +static int debug = DEBUG_NEED_INIT;
>> +
>> +bool debug_is(int lvl)
>> +{
>> +     return lvl <= debug_level();
>> +}
>> +
>> +int debug_level(void)
>> +{
>> +     if (debug != DEBUG_NEED_INIT)
>> +             return debug;
>> +     char *c = getenv("DEBUG");
>> +     if (!c) {
>> +             debug = CCAN_PR_LOG_DEFAULT_LEVEL;
>> +             return debug;
>> +     }
>> +
>> +     debug = atoi(c);
>> +     return debug;
>> +}
>> +
>> +void pr_log(char const fmt[static 1], ...)
>> +{
>> +     int level = INT_MIN;
>> +     if (fmt[0] == '<' && cisdigit(fmt[1]) && fmt[2] == '>')
>> +             level = fmt[1] - '0';
>> +
>> +     if (!debug_is(level))
>> +             return;
>> +
>> +     va_list va;
>> +     va_start(va, fmt);
>> +     vfprintf(stderr, fmt, va);
>> +     va_end(va);
>> +}
>> diff --git a/ccan/pr_log/pr_log.h b/ccan/pr_log/pr_log.h
>> new file mode 100644
>> index 0000000..628b826
>> --- /dev/null
>> +++ b/ccan/pr_log/pr_log.h
>> @@ -0,0 +1,61 @@
>> +/* Licensed under LGPLv2.1+ - see LICENSE file for details */
>> +#ifndef CCAN_PR_LOG_H_
>> +#define CCAN_PR_LOG_H_
>> +
>> +#include <stdbool.h>
>> +#include <ccan/compiler/compiler.h>
>> +
>> +#ifndef CCAN_PR_LOG_DEFAULT_LEVEL
>> +# define CCAN_PR_LOG_DEFAULT_LEVEL 6
>> +#endif
>> +
>> +#define LOG_EMERG  "<0>"
>> +#define LOG_ALERT  "<1>"
>> +#define LOG_CRIT   "<2>"
>> +#define LOG_ERROR  "<3>"
>> +#define LOG_WARN   "<4>"
>> +#define LOG_NOTICE "<5>"
>> +#define LOG_INFO   "<6>"
>> +#define LOG_DEBUG  "<7>"
>> +
>> +#ifndef CCAN_PR_LOG_DISABLE
>> +/**
>> + * pr_log - print output based on the given logging level
>> + *
>> + * Example:
>> + *
>> + *   pr_log(LOG_EMERG "something went terribly wrong\n");
>> + *   pr_log("N: notice this printout\n");
>> + */
>> +void PRINTF_FMT(1,2) pr_log(char const *fmt, ...);
>> +bool debug_is(int lvl);
>> +int debug_level(void);
>> +#else
>> +static PRINTF_FMT(1,2) inline void pr_log(char const *fmt, ...)
>> +{
>> +     (void)fmt;
>> +}
>> +static inline bool debug_is(int lvl) { (void)lvl; return false; }
>> +static inline int debug_level(void) { return 0; }
>> +#endif
>> +
>> +#define pr_emerg(...)  pr_log(LOG_EMERG  __VA_ARGS__)
>> +#define pr_alert(...)  pr_log(LOG_ALERT  __VA_ARGS__)
>> +#define pr_crit(...)   pr_log(LOG_CRIT   __VA_ARGS__)
>> +#define pr_error(...)  pr_log(LOG_ERROR  __VA_ARGS__)
>> +#define pr_warn(...)   pr_log(LOG_WARN   __VA_ARGS__)
>> +#define pr_notice(...) pr_log(LOG_NOTICE __VA_ARGS__)
>> +#define pr_info(...)   pr_log(LOG_INFO   __VA_ARGS__)
>> +#define pr_debug(...)  pr_log(LOG_DEBUG  __VA_ARGS__)
>> +
>> +#ifdef DEBUG
>> +# define pr_devel(...)  pr_debug(__VA_ARGS__)
>> +#else
>> +static PRINTF_FMT(1,2) inline void pr_check_printf_args(const char *fmt, ...)
>> +{
>> +     (void)fmt;
>> +}
>> +# define pr_devel(...) pr_check_printf_args(__VA_ARGS__)
>> +#endif
>> +
>> +#endif
>> diff --git a/ccan/pr_log/test/run.c b/ccan/pr_log/test/run.c
>> new file mode 100644
>> index 0000000..fe60bf2
>> --- /dev/null
>> +++ b/ccan/pr_log/test/run.c
>> @@ -0,0 +1,24 @@
>> +#include <ccan/pr_log/pr_log.h>
>> +#include <ccan/pr_log/pr_log.c>
>> +#include <ccan/tap/tap.h>
>> +
>> +int main(void)
>> +{
>> +     plan_tests(6);
>> +     ok1(!debug_is(4));
>> +     debug = 1;
>> +     ok1(debug_is(0));
>> +     ok1(debug_is(1));
>> +     ok1(!debug_is(2));
>> +     ok1(!debug_is(3));
>> +
>> +     pr_log("N: notice\n");
>> +     pr_log("-3: something important\n");
>> +     pr_log("1: less important\n");
>> +     pr_log("X: bad level is treated as HIGH\n");
>> +
>> +     debug = INT_MIN;
>> +     setenv("DEBUG", "1", 1);
>> +     ok1(debug_is(1));
>> +     return exit_status();
>> +}
>> --
>> 2.5.0
>>
>> _______________________________________________
>> ccan mailing list
>> ccan at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/ccan


More information about the ccan mailing list