[PATCH 1/2] log: new --log-func-loc option to enable function location in logs

[Aleksander Morgado]

Hey,

On Thu, May 18, 2017 at 12:43 PM, Aleksander Morgado
<aleksander at aleksander.es> wrote:
> The --debug output is a bit annoying right now because it includes the
> function location information by default. This information is most of
> the times not very useful, so just make it optional under a new
> command line argument for the daemon.

After Torsten's patches to include systemd journal support, which
already allow you to get function location information implicitly if
needed, I believe we could get rid of this in the default logging all
together.

So I'm going to suggest we skip this patch and we just completely
remove the function location in the default stdout/syslog logging,
unless someone has a strong opinion against it.

> ---
>  docs/man/ModemManager.8 | 3 +++
>  src/main.c              | 2 +-
>  src/mm-context.c        | 8 ++++++++
>  src/mm-context.h        | 1 +
>  src/mm-log.c            | 2 +-
>  5 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/docs/man/ModemManager.8 b/docs/man/ModemManager.8
> index f21208a3..4f4c4040 100644
> --- a/docs/man/ModemManager.8
> +++ b/docs/man/ModemManager.8
> @@ -61,6 +61,9 @@ Include absolute timestamps in the log output.
>  .TP
>  .B \-\-relative-timestamps
>  Include timestamps, relative to the start time of the daemon, in the log output.
> +.TP
> +.B \-\-log\-func\-loc
> +Include function location information (filename and line), in the log output.
>
>  .SH TEST OPTIONS
>  .TP
> diff --git a/src/main.c b/src/main.c
> index ada3927c..06172696 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -143,7 +143,7 @@ main (int argc, char *argv[])
>                         mm_context_get_log_file (),
>                         mm_context_get_timestamps (),
>                         mm_context_get_relative_timestamps (),
> -                       mm_context_get_debug (),
> +                       mm_context_get_log_func_loc (),
>                         &err)) {
>          g_warning ("Failed to set up logging: %s", err->message);
>          g_error_free (err);
> diff --git a/src/mm-context.c b/src/mm-context.c
> index c00fa544..0723b0eb 100644
> --- a/src/mm-context.c
> +++ b/src/mm-context.c
> @@ -23,6 +23,7 @@
>
>  static gboolean     version_flag;
>  static gboolean     debug;
> +static gboolean     log_func_loc;
>  static const gchar *log_level;
>  static const gchar *log_file;
>  static gboolean     show_ts;
> @@ -39,6 +40,7 @@ static const gchar *initial_kernel_events;
>  static const GOptionEntry entries[] = {
>      { "version", 'V', 0, G_OPTION_ARG_NONE, &version_flag, "Print version", NULL },
>      { "debug", 0, 0, G_OPTION_ARG_NONE, &debug, "Run with extended debugging capabilities", NULL },
> +    { "log-func-loc", 0, 0, G_OPTION_ARG_NONE, &log_func_loc, "Enable function location information on log messages", NULL },
>      { "log-level", 0, 0, G_OPTION_ARG_STRING, &log_level, "Log level: one of ERR, WARN, INFO, DEBUG", "[LEVEL]" },
>      { "log-file", 0, 0, G_OPTION_ARG_FILENAME, &log_file, "Path to log file", "[PATH]" },
>      { "timestamps", 0, 0, G_OPTION_ARG_NONE, &show_ts, "Show timestamps in log output", NULL },
> @@ -60,6 +62,12 @@ mm_context_get_debug (void)
>      return debug;
>  }
>
> +gboolean
> +mm_context_get_log_func_loc (void)
> +{
> +    return log_func_loc;
> +}
> +
>  const gchar *
>  mm_context_get_log_level (void)
>  {
> diff --git a/src/mm-context.h b/src/mm-context.h
> index 63a8ec4c..d43bc8c4 100644
> --- a/src/mm-context.h
> +++ b/src/mm-context.h
> @@ -27,6 +27,7 @@ void mm_context_init (gint argc,
>                        gchar **argv);
>
>  gboolean     mm_context_get_debug                 (void);
> +gboolean     mm_context_get_log_func_loc          (void);
>  const gchar *mm_context_get_log_level             (void);
>  const gchar *mm_context_get_log_file              (void);
>  gboolean     mm_context_get_timestamps            (void);
> diff --git a/src/mm-log.c b/src/mm-log.c
> index da424eed..bac0fcec 100644
> --- a/src/mm-log.c
> +++ b/src/mm-log.c
> @@ -117,7 +117,7 @@ _mm_log (const char *loc,
>          g_string_append_printf (msgbuf, "[%06ld.%06ld] ", secs, usecs);
>      }
>
> -    if (func_loc && log_level & LOGL_DEBUG)
> +    if (func_loc)
>          g_string_append_printf (msgbuf, "[%s] %s(): ", loc, func);
>
>      va_start (args, fmt);
> --
> 2.12.2
>



-- 
Aleksander
https://aleksander.es