* [PATCH] '.' causes wildcard expansion
@ 2012-06-11 6:56 Robert Mabee
2012-06-13 22:42 ` Robert Mabee
0 siblings, 1 reply; 5+ messages in thread
From: Robert Mabee @ 2012-06-11 6:56 UTC (permalink / raw)
To: grub-devel
Wildcard expansion in device name activates my floppy drive (PC BIOS).
I noticed that
echo .
also did so in 1.99, though there should be no expansion. Current
source fixes that case but
echo (.
still runs the floppy. split_path shouldn't treat . as a wildcard.
I believe the regcomp call in make_regex is incorrect because the
constant used is
intended only for a lower-level interface. The defines are more than a
little confusing.
=== modified file 'ChangeLog'
--- old/ChangeLog 2012-06-09 17:58:38 +0000
+++ new/ChangeLog 2012-06-11 06:32:58 +0000
@@ -1,3 +1,8 @@
+2012-06-11 Bob Mabee <rmabee@comcast.net>
+
+ * commands/wildcard.c (split_path): . is not a wildcard.
+ * (make_regex): RE_SYNTAX_GNU_AWK is not valid for regcomp cflags.
+
2012-06-09 Vladimir Serbinenko <phcoder@gmail.com>
* tests/grub_script_expansion.in: Explicitly tell grep that we handle
=== 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-10 23:16:02 +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;
@@ -181,7 +181,7 @@
if (ch == '\\' && end[1])
end++;
- else if (isregexop (ch))
+ else if (ch == '*') /* only wildcard currently implemented */
regex = 1;
else if (ch == '/' && ! regex)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] '.' causes wildcard expansion
2012-06-11 6:56 [PATCH] '.' causes wildcard expansion Robert Mabee
@ 2012-06-13 22:42 ` Robert Mabee
2012-06-18 11:03 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-06-19 12:19 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 2 replies; 5+ messages in thread
From: Robert Mabee @ 2012-06-13 22:42 UTC (permalink / raw)
To: grub-devel
Having second thoughts. Where I suggested taking out this call
> - else if (isregexop (ch))
> + else if (ch == '*') /* only wildcard currently implemented */
I should have just taken the extraneous '.' out of isregexop. That would
make it slightly simpler to add the remaining shell wildcards ('?' and '[')
when desired.
'[' already works, if there's also a * to trigger expansion, with the small
bug that special chars within [] still get escaped or translated so the set
may include an unexpected '.' or '\'.
Other regex chars also work, which probably should be counted as a
bug. I assume the eventual goal is to behave just like a common shell.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] '.' causes wildcard expansion
2012-06-13 22:42 ` Robert Mabee
@ 2012-06-18 11:03 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-06-19 12:19 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 0 replies; 5+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-06-18 11:03 UTC (permalink / raw)
To: grub-devel
[-- Attachment #1.1: Type: text/plain, Size: 952 bytes --]
On 14.06.2012 00:42, Robert Mabee wrote:
> Having second thoughts. Where I suggested taking out this call
>
>> - else if (isregexop (ch))
>> + else if (ch == '*') /* only wildcard currently implemented */
> I should have just taken the extraneous '.' out of isregexop.
It's used in other contexts as "needing escaping". So no, this part is fine
> That would
> make it slightly simpler to add the remaining shell wildcards ('?' and '[')
> when desired.
>
> '[' already works,
Not properly: try a[b*
> if there's also a * to trigger expansion, with the small
> bug that special chars within [] still get escaped or translated so the set
> may include an unexpected '.' or '\'.
>
> Other regex chars also work, which probably should be counted as a
> bug. I assume the eventual goal is to behave just like a common shell.
Please try attached patch
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: regex.diff --]
[-- Type: text/x-diff; name="regex.diff", Size: 1267 bytes --]
=== modified file 'grub-core/commands/wildcard.c'
--- grub-core/commands/wildcard.c 2012-06-08 20:54:21 +0000
+++ grub-core/commands/wildcard.c 2012-06-18 11:01:31 +0000
@@ -80,7 +80,7 @@ merge (char **dest, char **ps)
static inline int
isregexop (char ch)
{
- return grub_strchr ("*.\\", ch) ? 1 : 0;
+ return grub_strchr ("*.\\|+{}[]", ch) ? 1 : 0;
}
static char *
@@ -128,14 +128,22 @@ make_regex (const char *start, const cha
{
case '\\':
buffer[i++] = ch;
- if (*start != '\0')
+ if (*start == '*' || *start == '?')
buffer[i++] = *start++;
+ else
+ buffer[i++] = '\\';
break;
case '.':
case '(':
case ')':
case '@':
+ case '|':
+ case '+':
+ case '{':
+ case '}':
+ case '[':
+ case ']':
buffer[i++] = '\\';
buffer[i++] = ch;
break;
@@ -145,6 +153,10 @@ make_regex (const char *start, const cha
buffer[i++] = '*';
break;
+ case '?':
+ buffer[i++] = '.';
+ break;
+
default:
buffer[i++] = ch;
}
@@ -181,7 +193,7 @@ split_path (const char *str, const char
if (ch == '\\' && end[1])
end++;
- else if (isregexop (ch))
+ else if (ch == '*' || ch == '?')
regex = 1;
else if (ch == '/' && ! regex)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] '.' causes wildcard expansion
2012-06-13 22:42 ` Robert Mabee
2012-06-18 11:03 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2012-06-19 12:19 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-06-20 8:04 ` Robert Mabee
1 sibling, 1 reply; 5+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-06-19 12:19 UTC (permalink / raw)
To: grub-devel
[-- Attachment #1: Type: text/plain, Size: 1039 bytes --]
On 14.06.2012 00:42, Robert Mabee wrote:
> Having second thoughts. Where I suggested taking out this call
>
>> - else if (isregexop (ch))
>> + else if (ch == '*') /* only wildcard currently implemented */
> I should have just taken the extraneous '.' out of isregexop. That would
> make it slightly simpler to add the remaining shell wildcards ('?' and '[')
> when desired.
>
> '[' already works, if there's also a * to trigger expansion, with the small
> bug that special chars within [] still get escaped or translated so the set
> may include an unexpected '.' or '\'.
>
I've fixed these problems AFAICT now. Can you confirm?
> Other regex chars also work, which probably should be counted as a
> bug. I assume the eventual goal is to behave just like a common shell.
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] '.' causes wildcard expansion
2012-06-19 12:19 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2012-06-20 8:04 ` Robert Mabee
0 siblings, 0 replies; 5+ messages in thread
From: Robert Mabee @ 2012-06-20 8:04 UTC (permalink / raw)
To: grub-devel
On 06/19/2012 08:19 AM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> I've fixed these problems AFAICT now. Can you confirm?
I agree that many extraneous regex chars are now properly quoted so no more
functionality is exposed than is intended, now * and ?. I suggest also
quoting
^ and $ just for completeness. I can not explain why @ is quoted; it's
not mentioned
in the documentation for POSIX-compatible regex or in gnulib/regcomp.c.
wildcard_escape in script/execute.c quotes the wild chars and [ , which
is no longer
wild. This can lead to a surprising extra level of quoting, so for
example *'['*
becomes the useless regexp pattern ^.*\\\[.*$
It appears the only remaining use of isregexop is in make_dir, which
removes \
quoting only before regex chars, from path components that weren't wild.
If this quoting was inserted by wildcard_escape then it should be
removed similarly
to wildcard_unescape, which doesn't examine the quoted char.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-06-20 8:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-11 6:56 [PATCH] '.' causes wildcard expansion Robert Mabee
2012-06-13 22:42 ` Robert Mabee
2012-06-18 11:03 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-06-19 12:19 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-06-20 8:04 ` Robert Mabee
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.