Re: [PATCH 2/2 V2] MSVC: VS2013 comes with inttypes.h

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 29, 2016, 17:35 UTC
Message-ID: <xmqq4mbpyyp3.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <56FAB9FD.7080409@cs-ware.de>

Sven Strickroth <sven@cs-ware.de> writes:

> Signed-off-by: Sven Strickroth <email@cs-ware.de>
> ---

Ah, I didn't finish the comment I started to write here for the first round. My "Hmph" was meant to be followed by something like "Compared to 1/2 which clearly described what got changed, this does not say much to summarize what it did to help readers".

Re-reading the patch text, I tend to agree with you that the title "comes with inttypes.h" may be sufficient for readers to reason about the change, and it would not be necessary to explain that we can lose fallback definitions by including it.

Show 33 quoted lines

>  compat/mingw.h                  | 2 +-
>  compat/vcbuild/include/unistd.h | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 6b6d695..137f42e 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -415,7 +415,7 @@ int mingw_offset_1st_component(const char *path);
>  extern void build_libgit_environment(void);
>  extern const char *program_data_config(void);
>  #define git_program_data_config program_data_config
> -#ifndef __MINGW64_VERSION_MAJOR
> +#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER < 1800)
>  #define PRIuMAX "I64u"
>  #define PRId64 "I64d"
>  #else
> diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h
> index c65c2cd..b7cc48c 100644
> --- a/compat/vcbuild/include/unistd.h
> +++ b/compat/vcbuild/include/unistd.h
> @@ -45,11 +45,15 @@ typedef unsigned long long uintmax_t;
>  
>  typedef int64_t off64_t;
>  
> +#if !defined(_MSC_VER) || _MSC_VER < 1800
>  #define INTMAX_MIN  _I64_MIN
>  #define INTMAX_MAX  _I64_MAX
>  #define UINTMAX_MAX _UI64_MAX
>  
>  #define UINT32_MAX 0xffffffff  /* 4294967295U */
> +#else
> +#include<inttypes.h>

Somebody lost a SP here, it seems.

> +#endif
>  
>  #define STDIN_FILENO  0
>  #define STDOUT_FILENO 1