Re: [PATCH v4 1/4] Refactor skipping DOS drive prefixes

[Johannes Sixt <j6t@kdbg.org>]

Am 12.01.2016 um 08:57 schrieb Johannes Schindelin:
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 57ca477..b3e5044 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -361,7 +361,15 @@ HANDLE winansi_get_osfhandle(int fd);
>    * git specific compatibility
>    */
>   
> -#define has_dos_drive_prefix(path) (isalpha(*(path)) && (path)[1] == ':')
> +#define has_dos_drive_prefix(path) \
> +	(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
> +static inline int mingw_skip_dos_drive_prefix(char **path)
> +{
> +	int ret = has_dos_drive_prefix(*path);
> +	*path += ret;
> +	return ret;
> +}
> +#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix

This triggers

    CC alloc.o
In file included from git-compat-util.h:186,
                 from cache.h:4,
                 from alloc.c:12:
compat/mingw.h: In function 'mingw_skip_dos_drive_prefix':
compat/mingw.h:365: warning: implicit declaration of function 'isalpha'

when I build under the old MSYS environment. While I would understand
that the old MSYS environment is end-of-lifed and not worth your time
catering to, the error is still an indication of a problem.

Notice that mingw.h is #included in line 186 of git-compat-util.h,
isalpha is only (re-)defined much later in line 790. That would explain
the warning. What I do not understand is that you do not observe the
same warning in your MSYS2/MINGWxx environment. It would mean that
<ctype.h> is included somewhere.

At any rate, the resulting binary sometimes uses an isalpha
implementation other than the one provided in git-compat-util.h. The
result is most likely correct, but it is certainly not the intent,
is it?

I did not attempt to build with MSVC, but it is not unlikely that it
shows the same error.

I suggest to move the function definition out of line:

diff --git a/compat/mingw.c b/compat/mingw.c
index 10a51c0..0cebb61 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1915,6 +1915,13 @@ pid_t waitpid(pid_t pid, int *status, int options)
 	return -1;
 }
 
+int mingw_skip_dos_drive_prefix(char **path)
+{
+	int ret = has_dos_drive_prefix(*path);
+	*path += ret;
+	return ret;
+}
+
 int mingw_offset_1st_component(const char *path)
 {
 	char *pos = (char *)path;
diff --git a/compat/mingw.h b/compat/mingw.h
index 9b5db4e..2099b79 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -360,12 +360,7 @@ HANDLE winansi_get_osfhandle(int fd);
 
 #define has_dos_drive_prefix(path) \
 	(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
-static inline int mingw_skip_dos_drive_prefix(char **path)
-{
-	int ret = has_dos_drive_prefix(*path);
-	*path += ret;
-	return ret;
-}
+int mingw_skip_dos_drive_prefix(char **path);
 #define skip_dos_drive_prefix mingw_skip_dos_drive_prefix
 #define is_dir_sep(c) ((c) == '/' || (c) == '\\')
 static inline char *mingw_find_last_dir_sep(const char *path)