Hi Torsten,

On Mon, 26 Nov 2018, tbo...@web.de wrote:

> diff --git a/compat/cygwin.c b/compat/cygwin.c
> index b9862d606d..c4a10cb5a1 100644
> --- a/compat/cygwin.c
> +++ b/compat/cygwin.c
> @@ -1,19 +1,29 @@
>  #include "../git-compat-util.h"
>  #include "../cache.h"
>  
> +int cygwin_skip_dos_drive_prefix(char **path)
> +{
> +     int ret = has_dos_drive_prefix(*path);
> +     *path += ret;
> +     return ret;
> +}
> +
>  int cygwin_offset_1st_component(const char *path)
>  {
> -     const char *pos = path;
> +     char *pos = (char *)path;
> +
>       /* unc paths */
> -     if (is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
> +     if (!skip_dos_drive_prefix(&pos) &&
> +                     is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {

It takes a little folding and knotting of the brain to understand that
this `!skip_dos_drive_prefix(&pos)` has *nothing* to do with the comment
`unc paths` nor with the test whether the paths starts with two directory
separators.

As a consequence, I would highly suggest to turn this into:

        if (skip_dos_drive_prefix(&pos))
                ; /* absolute path with DOS drive prefix */
        /* unc paths */
        else if (is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {

That makes the code a lot easier to understand, and as a consequence a lot
harder to mess up in the future.

Thanks,
Dscho

>               /* skip server name */
> -             pos = strchr(pos + 2, '/');
> +             pos = strpbrk(pos + 2, "\\/");
>               if (!pos)
>                       return 0; /* Error: malformed unc path */
>  
>               do {
>                       pos++;
> -             } while (*pos && pos[0] != '/');
> +             } while (*pos && !is_dir_sep(*pos));
>       }
> +
>       return pos + is_dir_sep(*pos) - path;
>  }
> diff --git a/compat/cygwin.h b/compat/cygwin.h
> index 8e52de4644..46f29c0a90 100644
> --- a/compat/cygwin.h
> +++ b/compat/cygwin.h
> @@ -1,2 +1,34 @@
> +#define has_dos_drive_prefix(path) \
> +     (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
> +
> +
> +int cygwin_offset_1st_component(const char *path);
> +#define offset_1st_component cygwin_offset_1st_component
> +
> +
> +#define has_dos_drive_prefix(path) \
> +     (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
> +int cygwin_skip_dos_drive_prefix(char **path);
> +#define skip_dos_drive_prefix cygwin_skip_dos_drive_prefix
> +static inline int cygwin_is_dir_sep(int c)
> +{
> +     return c == '/' || c == '\\';
> +}
> +#define is_dir_sep cygwin_is_dir_sep
> +static inline char *cygwin_find_last_dir_sep(const char *path)
> +{
> +     char *ret = NULL;
> +     for (; *path; ++path)
> +             if (is_dir_sep(*path))
> +                     ret = (char *)path;
> +     return ret;
> +}
> +static inline void convert_slashes(char *path)
> +{
> +     for (; *path; path++)
> +             if (*path == '\\')
> +                     *path = '/';
> +}
> +#define find_last_dir_sep cygwin_find_last_dir_sep
>  int cygwin_offset_1st_component(const char *path);
>  #define offset_1st_component cygwin_offset_1st_component
> -- 
> 2.19.0.271.gfe8321ec05
> 
> 

Reply via email to