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

[Michael Schierl <schierlm@gmx.de>]

Hello Glenn,


Am 07.12.2021 um 10:17 schrieb Glenn Washburn:

>> maybe move the conditional before the assignment of ret?
>
> I'm understanding you to be suggesting to move the assignment of ret to
> after the if statement that follows it.

Correct.

> The only point I see is saving
> an assignment in the case that debug=all. Is there more to it?

No. It just bugged me to do a definitely never used assignment directly
before returning. Feel free to ignore it.


> Yep, this is an issue. It was intentional when I had the code only
> running in the case "all" was present, in which case skipping the
> condition if its the at the start of $debug is fine because the
> condition is already enabled. After changing to run also without "all"
> being present, this becomes an issue.

If "all" still was present I would probably have complained about
initializing it to +1 instead of +3 as the first three characters cannot
be a match if they are "all". :)

> I was using grub_iswordseparator in the original patch, but decided
> against it this round because the documentation states that separators
> are whitespace or comma.

Fair point. I don't think there are any common cases where this
backward-incompatible change really matters.


Regards,


Michael