Re: [PATCH 1/1] core: add "fatal-warnings" option to NM_DEBUG

[Lubomir Rintel <lrintel redhat com>]

Looks good.

On Tue, 2015-03-31 at 07:41 +0200, Thomas Haller wrote:
> NM already understands the command line argument --g-fatal-warnings
> which causes setting of g_log_set_always_fatal().
>
> Also interpret the "fatal-warnings" token in NM_DEBUG environment
> variable and in main.debug configuration setting.
>
> Usage hint: either set
>
>   $ export NM_DEBUG=RLIMIT_CORE,fatal-warnings
>
> or add the following section to NetworkManager.conf
>
>     [main]
>     debug=RLIMIT_CORE,fatal-warnings
> ---
>  man/NetworkManager.conf.xml.in |  8 +++++++-
>  src/main.c                     | 25 +++++++++++++++++--------
>  2 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/man/NetworkManager.conf.xml.in b/man/NetworkManager.conf.xml.in
> index 0fbf9d2..ae4ba88 100644
> --- a/man/NetworkManager.conf.xml.in
> +++ b/man/NetworkManager.conf.xml.in
> @@ -261,7 +261,13 @@ no-auto-default=*
>          values are supported:</para>
>          <para>
>            <literal>RLIMIT_CORE</literal>: set ulimit -c unlimited
> -          to write out core dumps.
> +          to write out core dumps. Beware, that a core dump can contain
> +          sensitive information such as passwords or configuration settings.
> +        </para>
> +        <para>
> +          <literal>fatal-warnings</literal>: set g_log_set_always_fatal()
> +          to core dump on warning messages from glib. This is equivalent
> +          to the --g-fatal-warnings command line option.
>          </para>
>          </listitem>
>        </varlistentry>
> diff --git a/src/main.c b/src/main.c
> index 2648a08..520a82d 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -160,11 +160,23 @@ parse_state_file (const char *filename,
>  }
>  
>  static void
> +_set_g_fatal_warnings ()
> +{
> +     GLogLevelFlags fatal_mask;
> +
> +     fatal_mask = g_log_set_always_fatal (G_LOG_FATAL_MASK);
> +     fatal_mask |= G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL;
> +     g_log_set_always_fatal (fatal_mask);
> +}
> +
> +static void
>  _init_nm_debug (const char *debug)
>  {
>       const guint D_RLIMIT_CORE = 1;
> +     const guint D_FATAL_WARNINGS = 2;
>       GDebugKey keys[] = {
>               { "RLIMIT_CORE", D_RLIMIT_CORE },
> +             { "fatal-warnings", D_FATAL_WARNINGS },
>       };
>       guint flags = 0;
>       const char *env = getenv ("NM_DEBUG");
> @@ -178,7 +190,7 @@ _init_nm_debug (const char *debug)
>       if (debug && strcasecmp (debug, "help") != 0)
>               flags |= g_parse_debug_string (debug,  keys, G_N_ELEMENTS (keys));
>  
> -     if (flags & D_RLIMIT_CORE) {
> +     if (NM_FLAGS_HAS (flags, D_RLIMIT_CORE)) {
>               /* only enable this, if explicitly requested, because it might
>                * expose sensitive data. */
>  
> @@ -188,6 +200,8 @@ _init_nm_debug (const char *debug)
>               };
>               setrlimit (RLIMIT_CORE, &limit);
>       }
> +     if (NM_FLAGS_HAS (flags, D_FATAL_WARNINGS))
> +             _set_g_fatal_warnings ();
>  }
>  
>  void
> @@ -270,13 +284,8 @@ main (int argc, char *argv[])
>       config_cli = nm_config_cmd_line_options_new ();
>       do_early_setup (&argc, &argv, config_cli);
>  
> -     if (global_opt.g_fatal_warnings) {
> -             GLogLevelFlags fatal_mask;
> -
> -             fatal_mask = g_log_set_always_fatal (G_LOG_FATAL_MASK);
> -             fatal_mask |= G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL;
> -             g_log_set_always_fatal (fatal_mask);
> -     }
> +     if (global_opt.g_fatal_warnings)
> +             _set_g_fatal_warnings ();
>  
>       if (global_opt.show_version) {
>               fprintf (stdout, NM_DIST_VERSION "\n");