[PATCH 1/2] mingw.h: permit arguments with side effects for is_dir_sep

2017-05-19 Thread Johannes Sixt
The implementation of is_dir_sep in git-compat-util.h uses an inline
function. Use one also for the implementation in compat/mingw.h to support
non-trivial argument expressions.

Signed-off-by: Johannes Sixt 
---
 compat/mingw.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index cdc112526a..5e8447019b 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -398,7 +398,11 @@ HANDLE winansi_get_osfhandle(int fd);
(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
 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 int mingw_is_dir_sep(int c)
+{
+   return c == '/' || c == '\\';
+}
+#define is_dir_sep mingw_is_dir_sep
 static inline char *mingw_find_last_dir_sep(const char *path)
 {
char *ret = NULL;
-- 
2.13.0.55.g17b7d13330



Re: [PATCH 1/2] mingw.h: permit arguments with side effects for is_dir_sep

2017-05-22 Thread Johannes Schindelin
Hi Hannes,


On Sat, 20 May 2017, Johannes Sixt wrote:

> The implementation of is_dir_sep in git-compat-util.h uses an inline
> function. Use one also for the implementation in compat/mingw.h to support
> non-trivial argument expressions.
> 
> Signed-off-by: Johannes Sixt 

I was a bit confused by the oneline. Maybe something like

mingw.h: avoid side effects with is_dir_sep()'s argument

Taking git-compat-util.h's cue (which uses an inline function
to back is_dir_sep()), let's use an inline function to back
also the Windows version of is_dir_sep(); This avoids problems
when calling the function with arguments that do more than just
provide a single character, e.g. incrementing a pointer. Example:

is_dir_sep(*(p++))

> diff --git a/compat/mingw.h b/compat/mingw.h
> index cdc112526a..5e8447019b 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -398,7 +398,11 @@ HANDLE winansi_get_osfhandle(int fd);
>   (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
>  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 int mingw_is_dir_sep(int c)
> +{
> + return c == '/' || c == '\\';
> +}
> +#define is_dir_sep mingw_is_dir_sep
>  static inline char *mingw_find_last_dir_sep(const char *path)
>  {
>   char *ret = NULL;

The patch is very good, of course.

ACK,
Dscho