grub-devel
[Top][All Lists]
Advanced

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

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


From: Michael Schierl
Subject: Re: [PATCH v2] misc: Allow selective disabling of debug facility names
Date: Tue, 7 Dec 2021 22:01:14 +0100
User-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.3.2

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



reply via email to

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