Re: [PATCH] mimgw: remove Compiler Warnings

From: Torsten Bögershausen <tboegi@web.de>
Date: Oct 9, 2024, 15:26 UTC
Message-ID: <20241009152628.GA16993@tb-raspi4>
In-Reply-To: <20241009103541.2887-1-soekkle@freenet.de>

On Wed, Oct 09, 2024 at 12:35:41PM +0200, Sören Krecker wrote:

Thanks for the patch. All looks sensible - 2 comments inline.

> Remove some complier warnings from msvc in compat/mingw.c for value truncation from 64 bit to 32 bit intigers.

small typo: compiler

Show 86 quoted lines

>
> Signed-off-by: Sören Krecker <soekkle@freenet.de>
> ---
>  compat/compiler.h               |  4 ++--
>  compat/mingw.c                  | 26 ++++++++++++++++----------
>  compat/vcbuild/include/unistd.h |  7 +++++++
>  3 files changed, 25 insertions(+), 12 deletions(-)
>
> diff --git a/compat/compiler.h b/compat/compiler.h
> index e9ad9db84f..e12e426404 100644
> --- a/compat/compiler.h
> +++ b/compat/compiler.h
> @@ -9,7 +9,7 @@
>
>  static inline void get_compiler_info(struct strbuf *info)
>  {
> -	int len = info->len;
> +	size_t len = info->len;
>  #ifdef __clang__
>  	strbuf_addf(info, "clang: %s\n", __clang_version__);
>  #elif defined(__GNUC__)
> @@ -27,7 +27,7 @@ static inline void get_compiler_info(struct strbuf *info)
>
>  static inline void get_libc_info(struct strbuf *info)
>  {
> -	int len = info->len;
> +	size_t len = info->len;
>
>  #ifdef __GLIBC__
>  	strbuf_addf(info, "glibc: %s\n", gnu_get_libc_version());
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 0e851ecae2..dca0816267 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -782,7 +782,7 @@ static inline void filetime_to_timespec(const FILETIME *ft, struct timespec *ts)
>   */
>  static int has_valid_directory_prefix(wchar_t *wfilename)
>  {
> -	int n = wcslen(wfilename);
> +	ssize_t n = wcslen(wfilename);
>
>  	while (n > 0) {
>  		wchar_t c = wfilename[--n];
> @@ -891,7 +891,7 @@ static int do_lstat(int follow, const char *file_name, struct stat *buf)
>   */
>  static int do_stat_internal(int follow, const char *file_name, struct stat *buf)
>  {
> -	int namelen;
> +	ssize_t namelen;
>  	char alt_name[PATH_MAX];
>
>  	if (!do_lstat(follow, file_name, buf))
> @@ -1274,7 +1274,8 @@ static const char *parse_interpreter(const char *cmd)
>  {
>  	static char buf[100];
>  	char *p, *opt;
> -	int n, fd;
> +	ssize_t n;
> +	int fd;
>
>  	/* don't even try a .exe */
>  	n = strlen(cmd);
> @@ -1339,7 +1340,7 @@ static char *path_lookup(const char *cmd, int exe_only)
>  {
>  	const char *path;
>  	char *prog = NULL;
> -	int len = strlen(cmd);
> +	size_t len = strlen(cmd);
>  	int isexe = len >= 4 && !strcasecmp(cmd+len-4, ".exe");
>
>  	if (strpbrk(cmd, "/\\"))
> @@ -1956,7 +1957,7 @@ char *mingw_getenv(const char *name)
>  #define GETENV_MAX_RETAIN 64
>  	static char *values[GETENV_MAX_RETAIN];
>  	static int value_counter;
> -	int len_key, len_value;
> +	size_t len_key, len_value;
>  	wchar_t *w_key;
>  	char *value;
>  	wchar_t w_value[32768];
> @@ -1968,7 +1969,9 @@ char *mingw_getenv(const char *name)
>  	/* We cannot use xcalloc() here because that uses getenv() itself */
>  	w_key = calloc(len_key, sizeof(wchar_t));
>  	if (!w_key)
> -		die("Out of memory, (tried to allocate %u wchar_t's)", len_key);
> +		die("Out of memory, (tried to allocate %"

Is there a reason to split the line like this ?

Show 12 quoted lines

> +			    PRIuMAX" wchar_t's)",
> +		    (uintmax_t)len_key);
>  	xutftowcs(w_key, name, len_key);
>  	/* GetEnvironmentVariableW() only sets the last error upon failure */
>  	SetLastError(ERROR_SUCCESS);
> @@ -1983,7 +1986,8 @@ char *mingw_getenv(const char *name)
>  	/* We cannot use xcalloc() here because that uses getenv() itself */
>  	value = calloc(len_value, sizeof(char));
>  	if (!value)
> -		die("Out of memory, (tried to allocate %u bytes)", len_value);
> +	    die("Out of memory, (tried to allocate %" PRIuMAX " bytes)",
> +	    (uintmax_t)len_value);

Indentation should be a TAB, not 4 spaces. And why this line-split ?

Show 38 quoted lines

>  	xwcstoutf(value, w_value, len_value);
>
>  	/*
> @@ -2001,7 +2005,7 @@ char *mingw_getenv(const char *name)
>
>  int mingw_putenv(const char *namevalue)
>  {
> -	int size;
> +	size_t size;
>  	wchar_t *wide, *equal;
>  	BOOL result;
>
> @@ -2011,7 +2015,8 @@ int mingw_putenv(const char *namevalue)
>  	size = strlen(namevalue) * 2 + 1;
>  	wide = calloc(size, sizeof(wchar_t));
>  	if (!wide)
> -		die("Out of memory, (tried to allocate %u wchar_t's)", size);
> +		die("Out of memory, (tried to allocate %" PRIuMAX " wchar_t's)",
> +		    (uintmax_t)size);
>  	xutftowcs(wide, namevalue, size);
>  	equal = wcschr(wide, L'=');
>  	if (!equal)
> @@ -3085,7 +3090,8 @@ static void maybe_redirect_std_handles(void)
>   */
>  int wmain(int argc, const wchar_t **wargv)
>  {
> -	int i, maxlen, exit_status;
> +	int exit_status;
> +	size_t i, maxlen;
>  	char *buffer, **save;
>  	const char **argv;
>
> diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h
> index 3a959d124c..ab3dc06709 100644
> --- a/compat/vcbuild/include/unistd.h
> +++ b/compat/vcbuild/include/unistd.h
> @@ -13,8 +13,15 @@ typedef _mode_t	mode_t;
>  #endif	/* Not _MODE_T_ */

Show 9 quoted lines

>
>  #ifndef _SSIZE_T_
> +#ifdef _WIN64
>  #define _SSIZE_T_
> +typedef __int64 _ssize_t;
> +#pragma message("Compiling on Win64")
> +#else
>  typedef long _ssize_t;
> +#endif // _AMD64

Is this needed, or is it a rest from a trial to use _ssize_t instead of ssize_t ?

Show 11 quoted lines

> +
> +
>
>  #ifndef	_OFF_T_
>  #define	_OFF_T_
>
> base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2
> --
> 2.39.5
>
>