[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