Re: [PATCH] compat/bswap.h: Simplify MSVC endianness detection

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Nov 8, 2020, 00:12 UTC
Message-ID: <20201108001254.GG6252@camp.crustytoothpaste.net>
In-Reply-To: <20201107234751.10661-1-dgurney99@gmail.com>

On 2020-11-07 at 23:47:51, Daniel Gurney wrote:

Show 21 quoted lines

> Modern MSVC or Windows versions don't support big-endian, so it's
> unnecessary to consider architectures when using it.
> 
> Signed-off-by: Daniel Gurney <dgurney99@gmail.com>
> ---
>  compat/bswap.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/compat/bswap.h b/compat/bswap.h
> index c0bb744adc..72f225eaa8 100644
> --- a/compat/bswap.h
> +++ b/compat/bswap.h
> @@ -74,7 +74,7 @@ static inline uint64_t git_bswap64(uint64_t x)
>  }
>  #endif
>  
> -#elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))
> +#elif defined(_MSC_VER)
>  
>  #include <stdlib.h>
>

The patch itself seems fine to me, of course. For the message, it may be valuable to mention that this enables the proper behavior for ARM64, which was your original goal. Also, by convention, we typically use a lower case letter after the colon in the summary.

And of course, thanks for the patch. I always have a soft spot for patches that improve architecture support.

-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US