Re: [PATCH 06/35] monitor: Create a log() macro.

Hannes Reinecke
2021-02-04T07:01:10Z

On 1/26/21 9:32 PM, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
>
> As this is a long running program, we need to make the log output
> configurable. First step: replace fprintf() by log(). The log level
> and printing of time stamps can be configured at run time using
> global variables. These will live in fabrics.c.
>
> Allow toggling function name printing at build time.
> Printing the function name is useful for development, but perhaps
> not desired for production.
> Put '#define LOG_FUNCNAME' before '#include "log.h"' to switch it on.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> log.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
> create mode 100644 log.h
>
> diff --git a/log.h b/log.h
> new file mode 100644
> index 0000000..2017731
> --- /dev/null
> +++ b/log.h
> @@ -0,0 +1,44 @@
> +#ifndef _LOG_H
> +#define _LOG_H
> +
> +#ifndef MAX_LOGLEVEL
> +# define MAX_LOGLEVEL LOG_DEBUG
> +#endif
> +#ifndef DEFAULT_LOGLEVEL
> +# define DEFAULT_LOGLEVEL LOG_NOTICE
> +#endif
> +
> +#ifdef LOG_FUNCNAME
> +#define _func_fmt "%s: "
> +#define _func_arg __func__
> +#else
> +#define _func_fmt "%s"
> +#define _func_arg ""
> +#endif
> +
> +extern int log_level;
> +extern bool log_timestamp;
> +#define _TIME_FMT "[%ld.%06ld] "
> +#define log(lvl, format, ...) \
> + do { \
> + int __lvl = (lvl); \
> + \
> + if (__lvl <= MAX_LOGLEVEL && __lvl <= log_level) { \
> + if (log_timestamp) { \
> + struct timespec __ts; \
> + \
> + clock_gettime(CLOCK_MONOTONIC, &__ts); \
> + fprintf(stderr, \
> + _TIME_FMT _func_fmt format, \
> + __ts.tv_sec, __ts.tv_nsec / 1000,\
> + _func_arg, \
> + ##__VA_ARGS__); \
> + } else { \
> + fprintf(stderr, _func_fmt format, \
> + _func_arg, \
> + ##__VA_ARGS__); \
> + }; \
> + } \
> + } while (0)
> +
> +#endif /* _LOG_H */
>
Urgh. Long macros are always horrible.
Can't you convert it into a function?

It might also be an idea to move this as the first function, as it's
arguably an extension to existing functionality, and not directly
related to the monitor.

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer