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

From: Johannes Sixt <j6t@kdbg.org>
Date: Jan 22, 2016, 18:50 UTC
Message-ID: <56A279DA.8080809@kdbg.org>
In-Reply-To: <05cb9e00756e8a364f972cd227804764f6a6380c.1452585382.git.johannes.schindelin@gmx.de>

Am 12.01.2016 um 08:57 schrieb Johannes Schindelin:

Show 18 quoted lines

> 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)