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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 22, 2017, 11:52 UTC
Message-ID: <alpine.DEB.2.21.1.1705221349200.3610@virtualbox>
In-Reply-To: <69bafb7c9e08ba837a9b92573bef165a62c5bf92.1495261020.git.j6t@kdbg.org>

Hi Hannes,

On Sat, 20 May 2017, Johannes Sixt wrote:

Show 5 quoted lines

> 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 <j6t@kdbg.org>

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

Show 17 quoted lines

> 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