grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3] misc: Allow selective disabling of debug facility names


From: Daniel Kiper
Subject: Re: [PATCH v3] misc: Allow selective disabling of debug facility names
Date: Thu, 16 Dec 2021 18:49:28 +0100
User-agent: NeoMutt/20170113 (1.7.2)

On Fri, Dec 10, 2021 at 02:13:52AM -0600, Glenn Washburn wrote:
> Sometimes you know only know which debug logging facility names you want to

s/only know/only/

> turn off, not necessarily all the ones you want enabled. This patch allows
> the debug string to contain facility names in the $debug variable which are
> prefixed with a "-" to disable debug log messages for that conditional. Say
> you want all debug logging on except for btrfs and scripting, then do:
>  "set debug=all,-btrfs,-scripting"
>
> Note, that only the last occurence of the facility name with or without a
> leading "-" is considered. So simply appending ",-facilityname" to the
> $debug variable will disable that conditional. To illustrate, the command
> "set debug=all,-btrfs,-scripting,btrfs" will enable btrfs.
>
> Also, add documentation explaining this new behavior.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
> Changes since v2:
> * Fix issue where a facility at the start of the string wouldn't be matched
>
> ---
> Range-diff against v2:
> 1:  ef2662d0b ! 1:  31439a7fb misc: Allow selective disabling of debug 
> facility names
>     @@ grub-core/kern/misc.c: __attribute__ ((alias("grub_printf")));
>      -    return 1;
>      +  if (grub_strword (debug, "all"))
>      +    {
>     -+      ret = 1;
>      +      if (debug[3] == '\0')
>      +        return 1;
>     ++      ret = 1;
>      +    }
>
>      -  return 0;
>      +  clen = grub_strlen (condition);
>     -+  found = debug;
>     ++  found = debug-1;
>      +  while(1)
>      +    {
>      +      found = grub_strstr (found+1, condition);
>     @@ grub-core/kern/misc.c: __attribute__ ((alias("grub_printf")));
>      +        continue;
>      +
>      +      /*
>     -+       * If found condition is prefixed with '-' and the start is on a 
> word
>     ++       * If found condition is at the start of debug, then enable 
> debug. Else
>     ++       * if found condition is prefixed with '-' and the start is on a 
> word
>      +       * boundary, then disable debug. Otherwise, if the start is on a 
> word
>     -+       * boundary, enable debug. If neither, ignore.
>     ++       * boundary, enable debug. If none of these cases, ignore.
>      +       */
>     -+      if (*(found-1) == '-' && ((found == debug + 1) || (*(found-2) == 
> ','
>     ++      if (found == debug)
>     ++        ret = 1;
>     ++      else if (*(found-1) == '-' && ((found == debug + 1) || 
> (*(found-2) == ','
>      +                               || grub_isspace (*(found-2)))))
>      +        ret = 0;
>      +      else if (*(found-1) == ',' || grub_isspace (*(found-1)))
>
>  docs/grub.texi        | 13 ++++++++++---
>  grub-core/kern/misc.c | 43 +++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 49 insertions(+), 7 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 99d0a0149..d13aa6600 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3388,9 +3388,16 @@ processed by commands @command{configfile} 
> (@pxref{configfile}) or @command{norm
>  @subsection debug
>
>  This variable may be set to enable debugging output from various components
> -of GRUB.  The value is a list of debug facility names separated by
> -whitespace or @samp{,}, or @samp{all} to enable all available debugging
> -output. The facility names are the first argument to grub_dprintf. Consult
> +of GRUB. The value is an ordered list of debug facility names separated by
> +whitespace or @samp{,}. If the special facility names @samp{all} is present

s/names/name/

> +then debugging output of all facility names is enabled at the start of
> +processing the value of this variable. A facility's debug output can then be
> +disabled by prefixing its name with a @samp{-}. The last occurence facility
> +name with or without a leading @samp{-} takes precendent over any previous
> +occurence. This allows the easy enabling or disabling of facilities by
> +appending a @samp{,} and then the facility name with or without the leading
> +@samp{-}, which will preserve the state of the rest of the facilities.
> +The facility names are the first argument to grub_dprintf. Consult the
>  source for more details.
>
>
> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
> index 11b8592c8..fda69fe00 100644
> --- a/grub-core/kern/misc.c
> +++ b/grub-core/kern/misc.c
> @@ -162,16 +162,51 @@ __attribute__ ((alias("grub_printf")));
>  int
>  grub_debug_enabled (const char * condition)
>  {
> -  const char *debug;
> +  const char *debug, *found;
> +  grub_size_t clen;
> +  int ret = 0;
>
>    debug = grub_env_get ("debug");
>    if (!debug)
>      return 0;
>
> -  if (grub_strword (debug, "all") || grub_strword (debug, condition))
> -    return 1;
> +  if (grub_strword (debug, "all"))
> +    {
> +      if (debug[3] == '\0')
> +     return 1;
> +      ret = 1;
> +    }
>
> -  return 0;
> +  clen = grub_strlen (condition);
> +  found = debug-1;
> +  while(1)
> +    {
> +      found = grub_strstr (found+1, condition);
> +
> +      if (found == NULL)
> +     break;
> +
> +      /* Found condition is not a whole word, so ignore it */
> +      if (*(found + clen) != '\0' && *(found + clen) != ','
> +      && !grub_isspace (*(found + clen)))
> +     continue;
> +
> +      /*
> +       * If found condition is at the start of debug, then enable debug. Else
> +       * if found condition is prefixed with '-' and the start is on a word
> +       * boundary, then disable debug. Otherwise, if the start is on a word
> +       * boundary, enable debug. If none of these cases, ignore.
> +       */
> +      if (found == debug)
> +     ret = 1;
> +      else if (*(found-1) == '-' && ((found == debug + 1) || (*(found-2) == 
> ','
> +                            || grub_isspace (*(found-2)))))
> +     ret = 0;
> +      else if (*(found-1) == ',' || grub_isspace (*(found-1)))

I think this could be merged with first if.

Additionally, please add missing spaces around "+" and "-" operators.

> +     ret = 1;
> +    }
> +
> +  return ret;
>  }

Daniel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]