Re: Release of 2.00~rc1

[Robert Mabee]

On 06/25/2012 02:23 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 24.06.2012 09:29, Robert Mabee wrote:
>
> > Missed this one, where the symbol belongs to some other interface,
> > but has a value close enough to require an insane test (a filename
> > containing a newline) to get incorrect results:
> >
> > === modified file 'grub-core/commands/wildcard.c'
> > --- old/grub-core/commands/wildcard.c 2012-06-08 20:54:21 +0000
> > +++ new/grub-core/commands/wildcard.c 2012-06-24 06:55:33 +0000
> > @@ -153,7 +153,7 @@
> > buffer[i] = '\0';
> > grub_dprintf ("expand", "Regexp is %s\n", buffer);
> >
> > - if (regcomp (regexp, buffer, RE_SYNTAX_GNU_AWK))
> > + if (regcomp (regexp, buffer, REG_EXTENDED))
> > {
> > grub_free (buffer);
> > return 1;
> REG_EXTENDED isn't even a syntax type but is just refined to 1. Valid
> syntaxes are:
>
> RE_SYNTAX_EMACS
> RE_SYNTAX_AWK
> RE_SYNTAX_GNU_AWK                                               
> RE_SYNTAX_POSIX_AWK                                             RE_SYNTAX_GREP
> RE_SYNTAX_EGREP                                         RE_SYNTAX_POSIX_EGREP   
>                                         RE_SYNTAX_ED
> RE_SYNTAX_SED
> RE_SYNTAX_POSIX_BASIC                                           
> RE_SYNTAX_POSIX_MINIMAL_BASIC                           
> RE_SYNTAX_POSIX_EXTENDED                                        
> RE_SYNTAX_POSIX_MINIMAL_EXTENDED                                
>
> Additionally no real testcase was ever demonstrated.
Looking at the code and "man regex" (not binding on Grub, but describes
the code that was imported), it appears regcomp takes the OR of any of
REG_EXTENDED, REG_ICASE, REG_NEWLINE, REG_NOSUB, all defined by
POSIX. RE_SYNTAX_* are valid args to re_set_syntax or re_compile_internal,
defined by GNU.

RE_SYNTAX_GNU_AWK happens to have the bits REG_EXTENDED, REG_NEWLINE,
and REG_NOSUB set. The last has no effect because wildcard.c doesn't pass
the array to collect subexpression matches. REG_EXTENDED is usually used
because the alternative ("basic") syntax is called obsolete. However, 
the basic
syntax has many fewer special characters so would save code (to quote them)
in wildcard.c. Probably REG_ICASE should be set according to the fs case
insensitivity.

REG_NEWLINE causes incorrect results when a filename contains a newline,
both because a wild char won't match the newline, and because the generated
pattern anchors ^$ can match adjacent to the newline. For example, after
echo foo >'/foo
bar'
(shell, OS, fs permitting) Grub will fail to show this file with ls /f*r 
but will show
it with /*o or /b* (as well as the expected /*r and /f*).

I support your choice not to include this in a code freeze, since it is 
so unlikely
to affect anyone. I only wanted to make sure it didn't get lost.