Re: [patch 1/2] auto-load safe-path: Permit shell wildcards

[Eli Zaretskii <eliz@gnu.org>]

> Date: Sun, 17 Jun 2012 21:37:03 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> BTW I have verified that MS-Windows native function GetFullPathName transforms
> '/' -> '\':
> 	auto-load: Using directory "c:/gdb".
> 	auto-load: And canonicalized as "c:\gdb".
> I tried it on Windows8-ReleasePreview-64bit-English.iso but with the
> transformation by gdb_filename_fnmatch GDB does no longer need to depend on it
> (I am for example not sure if older MS-Windows also behaved that way).

GetFullPathName always converted the slashes to backslashes, since it
returns the native OS file name.

Not sure how this is related to the issue at hand, though.

> +/* Return 1 if FILENAME matches PATTERN or if FILENAME belongs to
> +   a subdirectory permitted by PATTERN.  Return 0 otherwise.
        ^^^^^^^^^^^^
Why "subdirectory" and not "directory"?

Or did you mean "... or if FILENAME resides in a subdirectory of a
directory that matches PATTERN"?

> +   gdb_realpath normalization is never done here.  */
>  
>  static ATTRIBUTE_PURE int
> -filename_is_in_dir (const char *filename, const char *dir)
> +filename_is_in_pattern (const char *filename_orig, const char *pattern_orig)

The arguments are named differently from what the commentary says.
(Do you really need the "_orig" suffix here?)

> +  /* Trim trailing slashes ("/") from PATTERN.  */
> +  while (pattern_len && IS_DIR_SEPARATOR (pattern[pattern_len - 1]))
> +    pattern_len--;
> +  pattern[pattern_len] = '\0';

Wouldn't this will do the wrong thing with a pattern such as "d:/"?
(I'm not sure whether it will DTRT with "/" as well.)

> +      /* Trim trailing slashes ("/").  */
> +      while (filename_len && IS_DIR_SEPARATOR (filename[filename_len - 1]))
> +	filename_len--;
> +      filename[filename_len] = '\0';

Same here, I think.

>  @value{GDBN} provides the @samp{set auto-load safe-path} setting to list
>  directories trusted for loading files not explicitly requested by user.
> +Each directory can be also shell wildcard pattern.
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"... can also be a shell wildcard."

> +Each directory can be also shell wildcard pattern;

Likewise.

> +                                                  '*' matches only single
> +component, it does not match across directory separator.

"... @samp{*} matches a single component ..."

Should we describe all of the wildcard meta-characters, not just '*'?

The documentation part is OK with these changes.

Thanks.