Am 12.01.2011 11:57, schrieb Michael Tokarev:
> Currently the two routines tries to "understand" and skip <protocol>:
> prefix in path arguments are path_combine() and path_is_absolute()
> (the latter isn't used anywhere but in the former). This is wrong,
> since notion of absolute path is, at least, protocol-specific.
>
> The implementation is more wrong on windows where even non-absolute
> paths but with drive name (d:foo) should be treated as absolute, as
> in, one can't combine, say, c:\bar with d:foo forming c:\foo as
> path_combine() currently does.
>
> Introduce isslash() macro that works correctly on both windows and
> unix, use it in is_windows_drive_prefix() (since any form of slash
> can be used in constructs like //./),
You're saying that \/.\ is a valid format for it? Wow...
> remove path_is_absolute() and
> hardcode the trivial (but right) condition in path_combine(), and
> simplify path_combine() further by removing <protocol>: handling
> and unifying shash searching.
>
> Signed-off-by: Michael Tokarev <m...@tls.msk.ru>
> ---
> block.c | 72 +++++++++++++++++++++-----------------------------------------
> 1 files changed, 25 insertions(+), 47 deletions(-)
>
> diff --git a/block.c b/block.c
> index 42d6ff1..31a821d 100644
> --- a/block.c
> +++ b/block.c
> @@ -71,6 +71,9 @@ static BlockDriverState *bs_snapshots;
> static int use_bdrv_whitelist;
>
> #ifdef _WIN32
> +
> +#define isslash(c) ((c) == '/' || (c) == '\\')
> +
> static int is_windows_drive_prefix(const char *filename)
> {
> return (((filename[0] >= 'a' && filename[0] <= 'z') ||
> @@ -83,11 +86,17 @@ int is_windows_drive(const char *filename)
> if (is_windows_drive_prefix(filename) &&
> filename[2] == '\0')
> return 1;
> - if (strstart(filename, "\\\\.\\", NULL) ||
> - strstart(filename, "//./", NULL))
> - return 1;
> + if (isslash(filename[0] && isslash(filename[1]) &&
Missing bracket, doesn't even compile.
> + filename[2] == '.' && isslash(filename[3]))
> + return 1; /* special case: windows device "//./" */
Please add curly braces.
> return 0;
> }
> +
> +#else
> +
> +#define isslash(c) ((c) == '/')
> +#define is_windows_drive_prefix(filename) (0)
Please make this a function, for consistency and type checking. The
compiler is going to inline it anyway.
> +
> #endif
>
> /* check if the path starts with "<protocol>:"
> @@ -115,61 +124,30 @@ static char *path_has_protocol(const char *path)
> (char*)p : NULL;
> }
>
> -int path_is_absolute(const char *path)
> -{
> - const char *p;
> -#ifdef _WIN32
> - /* specific case for names like: "\\.\d:" */
> - if (*path == '/' || *path == '\\')
> - return 1;
> -#endif
> - p = strchr(path, ':');
> - if (p)
> - p++;
> - else
> - p = path;
> -#ifdef _WIN32
> - return (*p == '/' || *p == '\\');
> -#else
> - return (*p == '/');
> -#endif
> -}
> -
> /* if filename is absolute, just copy it to dest. Otherwise, build a
> - path to it by considering it is relative to base_path. URL are
> - supported. */
> + path to it by considering it is relative to base_path.
> + This is really about filenames not URLs - we don't support
> + <protocol>: prefix in filename since it makes no sense, especially
> + if the protocol in base_path is not the same as in filename.
> + */
> void path_combine(char *dest, int dest_size,
> const char *base_path,
> const char *filename)
> {
> - const char *p, *p1;
> + const char *p;
> int len;
>
> if (dest_size <= 0)
> return;
> - if (path_is_absolute(filename)) {
> + /* on windows, d:filename should be treated as absolute too */
> + if (isslash(filename[0]) || is_windows_drive_prefix(filename)) {
> pstrcpy(dest, dest_size, filename);
> } else {
> - p = strchr(base_path, ':');
> - if (p)
> - p++;
> - else
> - p = base_path;
> - p1 = strrchr(base_path, '/');
> -#ifdef _WIN32
> - {
> - const char *p2;
> - p2 = strrchr(base_path, '\\');
> - if (!p1 || p2 > p1)
> - p1 = p2;
> - }
> -#endif
> - if (p1)
> - p1++;
> - else
> - p1 = base_path;
> - if (p1 > p)
> - p = p1;
> + /* find last slash */
> + p = base_path + strlen(base_path);
> + while(p >= base_path && !isslash(*p))
> + --p;
> + p++;
Please add braces.
> len = p - base_path;
> if (len > dest_size - 1)
> len = dest_size - 1;
This changes the semantics to throw away the protocol. For example
fat:foo combined with bar would have resulted in fat:bar previously and
results in bar now.
Probably not a problem. path_combine gets even worse if filename has a
protocol. It's completely unclear what it's supposed to do with
protocols anyway.
Kevin
