On Tue, 19 Oct 2021 21:10:27 +0200 Michael Schierl <schie...@gmx.de> wrote:
> > Hello, > > > Am 19.10.2021 um 08:47 schrieb Glenn Washburn: > > > > Note, only the first occurance of the conditional is searched for to > > determine if the conditional is enabled. So simply appending ",-conditional" > > to the $debug variable may not disable that conditional, if, for example, > > the conditional is already present. To illustrate, the command > > "set debug=all,btrfs,-scripting,-btrfs" will not disable btrfs. > > A sligtly bigger problem of the implementation is that > > > set debug=all,-cryptodisk,-crypt,-disk > > will only exclude cryptodisk, as the other two are found as substrings > first. Same for e.g. fs,xfs or ata,pata. Hmm, good catch. > > But it is probably the best compromise (both performance- and code-size > wise) when considering grub's built-in string functions, when parsing > this exact command syntax. Yes, I was trying to avoid a loop, but looks like it really should have one. I think this patch needs another version. > > As there is no real advantage of both including and excluding > conditionals (with the exception of "all"), a pragmatic approach may be > to change the syntax, so that instead of > > set debug=all,-scripting,-btrfs > > one has to specify > > set debug=-scripting,btrfs > > (i.e. if the first character of the variable is a -, the rest is a list > of excluded debug conditionals). So after comparing the first character, > you can skip it and use grub_strword on the rest. This is not a bad idea, and I like that it makes things simpler. I don't feel great about the implied "all" though and I think the syntax may be confusing for the user at first. Performance wise, I'd say its about the same as me adding a loop to this patch. I'm not opposed to this, but I'd want Daniel or others to chime in on their thoughts. Thanks for taking a look! Glenn _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
