threads / patch / 62292

patchmimgw: remove Compiler Warnings

Subject: [PATCH] mimgw: remove Compiler Warnings

## tl;dr

16 messages between Oct 9, 2024 and Oct 17, 2024. Diffs are folded; open one to read it.

replies: 15people: 5as markdown or json

Sören Krecker· Oct 9, 2024, 10:35 UTC · lore
Remove some complier warnings from msvc in compat/mingw.c for value truncation from 64 bit to 32 bit intigers.
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(-)
Show changes to 3 files +25 −12

compat/compiler.h, compat/mingw.c, compat/vcbuild/include/unistd.h

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 %"
+			    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);
 	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_ */
 
 #ifndef _SSIZE_T_
+#ifdef _WIN64
 #define _SSIZE_T_
+typedef __int64 _ssize_t;
+#pragma message("Compiling on Win64")
+#else
 typedef long _ssize_t;
+#endif // _AMD64
+
+
 
 #ifndef	_OFF_T_
 #define	_OFF_T_

base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2
-- 
2.39.5
Torsten Bögershausen· Oct 9, 2024, 15:26 UTC · re: Sören Krecker · lore

Re: [PATCH] mimgw: remove Compiler Warnings

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
>
>
Phillip Wood· Oct 9, 2024, 16:13 UTC · re: Sören Krecker · lore

Re: [PATCH] mimgw: remove Compiler Warnings

Hi Sören
On 09/10/2024 11:35, Sören Krecker wrote:
> Remove some complier warnings from msvc in compat/mingw.c for value truncation from 64 bit to 32 bit intigers.

Thanks for working on this, it is a useful improvement. It would be helpful to explain the choice of signed/unsigned type in each case to help reviewers check that the conversion is correct. I've looked through the code and there are a couple I'm not sure about. I'd also echo Torsten's code formatting comments.

Show 29 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());

These two look straight forward - we save info->len at the start of the function and see if it has changed at the end.

Show 10 quoted lines
> 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);
This is ssize_t because n maybe negative as seen in the context below - good
Show 9 quoted lines
>   
>   	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;
Looking at this function I can't see why this is ssize_t rather than size_t
Show 6 quoted lines
> @@ -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;

This is ssize_t because we assign the return value of read() to n which maybe negative - good

Show 6 quoted lines
> @@ -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);
This looks good we're holding the length of a string
Show 9 quoted lines
>   	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;
This looks good too, they're holding the length of a string
Show 6 quoted lines
> @@ -2001,7 +2005,7 @@ char *mingw_getenv(const char *name)
>   
>   int mingw_putenv(const char *namevalue)
>   {
> -	int size;
> +	size_t size;
This looks correct - another string length
Show 7 quoted lines
> @@ -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;

"i" loops over 0..argc so I think we want to keep it as an int. maxlen is a string length so should be size_t.

Best Wishes
Phillip
Show 25 quoted lines
>   	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_ */
>   
>   #ifndef _SSIZE_T_
> +#ifdef _WIN64
>   #define _SSIZE_T_
> +typedef __int64 _ssize_t;
> +#pragma message("Compiling on Win64")
> +#else
>   typedef long _ssize_t;
> +#endif // _AMD64
> +
> +
>   
>   #ifndef	_OFF_T_
>   #define	_OFF_T_
> 
> base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2
Sören Krecker· Oct 9, 2024, 17:13 UTC · re: Phillip Wood · lore
Hi Torsten, Hi Phillip,

I have tried to implement your comments. I add som comments after the verable to explain what there contains. I try to do less a possiable changes, the _ssize_t was there before and I change the type on win 64.

Best regards,
Sören Krecker
Sören Krecker (1):
  mimgw: remove Compiler Warnings
 compat/compiler.h               |  4 ++--
 compat/mingw.c                  | 25 +++++++++++++++----------
 compat/vcbuild/include/unistd.h |  4 ++++
 3 files changed, 21 insertions(+), 12 deletions(-)
base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2
-- 
2.39.5
Sören Krecker· Oct 9, 2024, 17:13 UTC · re: Sören Krecker · lore

[PATCH v2 1/1] [PATCH] mimgw: remove Compiler Warnings

Remove some compiler warnings from msvc in compat/mingw.c for value truncation from 64 bit to 32 bit intigers.
Signed-off-by: Sören Krecker <soekkle@freenet.de>
---
 compat/compiler.h               |  4 ++--
 compat/mingw.c                  | 25 +++++++++++++++----------
 compat/vcbuild/include/unistd.h |  4 ++++
 3 files changed, 21 insertions(+), 12 deletions(-)
Show changes to 3 files +21 −12

compat/compiler.h, compat/mingw.c, compat/vcbuild/include/unistd.h

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..5293f4cdae 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); /*can become negative*/
 
 	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;
+	size_t namelen; /* contains length of a string*/
 	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; /* read() can return negativ values */
+	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; /* lengt of strings */
 	wchar_t *w_key;
 	char *value;
 	wchar_t w_value[32768];
@@ -1968,7 +1969,8 @@ 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 %"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 +1985,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);
 	xwcstoutf(value, w_value, len_value);
 
 	/*
@@ -2001,7 +2004,7 @@ char *mingw_getenv(const char *name)
 
 int mingw_putenv(const char *namevalue)
 {
-	int size;
+	size_t size; /* lengt of a string */
 	wchar_t *wide, *equal;
 	BOOL result;
 
@@ -2011,7 +2014,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 +3089,8 @@ static void maybe_redirect_std_handles(void)
  */
 int wmain(int argc, const wchar_t **wargv)
 {
-	int i, maxlen, exit_status;
+	int i, exit_status;
+	size_t maxlen; /*contains length os arguments*/
 	char *buffer, **save;
 	const char **argv;
 
diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h
index 3a959d124c..1c0096ab21 100644
--- a/compat/vcbuild/include/unistd.h
+++ b/compat/vcbuild/include/unistd.h
@@ -14,7 +14,11 @@ typedef _mode_t	mode_t;
 
 #ifndef _SSIZE_T_
 #define _SSIZE_T_
+#ifdef _WIN64
+typedef __int64 _ssize_t;
+#else
 typedef long _ssize_t;
+#endif // _AMD64
 
 #ifndef	_OFF_T_
 #define	_OFF_T_
-- 
2.39.5
Junio C Hamano· Oct 9, 2024, 18:20 UTC · re: Sören Krecker · lore

Re: [PATCH v2 1/1] [PATCH] mimgw: remove Compiler Warnings

Sören Krecker <soekkle@freenet.de> writes:
> Remove some compiler warnings from msvc in compat/mingw.c for value truncation from 64 bit to 32 bit intigers.
An overly long line?
Show 10 quoted lines
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 0e851ecae2..5293f4cdae 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); /*can become negative*/

Aside from the malformed comment ("/* can become negative */" with spaces would have been OK), I am not sure where it can become negative, unless wcslen() is allowed to return a negative value to signal some kind of error (in which case, the comment should say that), which is not the case.

The loop body in the post-context of this hunk looks like
>  	while (n > 0) {
>  		wchar_t c = wfilename[--n];
		... 'n' is not written anywhere else in this loop ...
	}

so an 'n' that is not negative before entering the loop can never become negative by what the loop body does.

Show 6 quoted lines
> @@ -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;
> +	size_t namelen; /* contains length of a string*/

Indeed, this receives the return value of strlen(). I am not sure if this comment is necessary, though. Just like you omitted any comment on size_t variables that receives .len in a strbuf, its correctness is rather obvious.

Show 6 quoted lines
> @@ -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; /* read() can return negativ values */
The word is "negative".

But 'n' is also used to receive the result of strlen(). A kosher rewrite may be to split it into two separate variables,

	size_t cmdlen = strlen(cmd);
	ssize_t bytes_read = read(fd, buf, sizeof(buf)-1);
Show 6 quoted lines
> @@ -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; /* lengt of strings */
"length".

Again given "size_t strlen(const char *)", this may be sufficiently obvious.

Show 6 quoted lines
> @@ -2001,7 +2004,7 @@ char *mingw_getenv(const char *name)
>  
>  int mingw_putenv(const char *namevalue)
>  {
> -	int size;
> +	size_t size; /* lengt of a string */
Ditto.
Show 7 quoted lines
> @@ -3085,7 +3089,8 @@ static void maybe_redirect_std_handles(void)
>   */
>  int wmain(int argc, const wchar_t **wargv)
>  {
> -	int i, maxlen, exit_status;
> +	int i, exit_status;
> +	size_t maxlen; /*contains length os arguments*/
Missing SP around the words.

Again, given "size_t wcslen(const wchar_t *)", it may be obvious to readers.

Show 7 quoted lines
> diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h
> ...
> +#ifdef _WIN64
> +typedef __int64 _ssize_t;
> +#else
>  typedef long _ssize_t;
> +#endif // _AMD64

It is a bit unusual that "#ifdef X" is not closed with "#endif /* X */". Some folks write "#endif /* !X */" but what I am wondering about is a mismatch between _WIN64 and _AMD64.

Phillip Wood· Oct 10, 2024, 08:59 UTC · re: Sören Krecker · lore

Re: [PATCH v2 1/1] [PATCH] mimgw: remove Compiler Warnings

Hi Sören
On 09/10/2024 18:13, Sören Krecker wrote:
> Remove some compiler warnings from msvc in compat/mingw.c for value truncation from 64 bit to 32 bit intigers.

Thanks for re-rolling, I think "Fix some compiler warnings" would be clearer than "Remove", also "integers" is misspelt. As Junio said we fold our commit messages at 72 characters. When I said it would be helpful to explain the choice of signed/unsigned I meant an explanation in the commit message, not code comments. I agree with Junio that the remaining ssize_t should be a size_t so the commit message could say something like

     Use size_t instead of int as all of the changed variables hold the
     result of strlen() or wcslen() which cannot be negative.

It would also be helpful to explain in the commit message the changes to _ssize_t

Show 5 quoted lines
> +#ifdef _WIN64
> +typedef __int64 _ssize_t;
> +#else
>   typedef long _ssize_t;
> +#endif // _AMD64

Please note that we do not use "//" comments so this should be "/* _WIN64 */" so that the comment matches the opening #ifdef

Thanks for working on this
Phillip
Sören Krecker· Oct 10, 2024, 10:29 UTC · re: Phillip Wood · lore

[PATCH] [PATCH] mimgw: Remove Compiler Warnings

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

Use size_t instead of int as all of the changed variables hold the result of strlen() or wcslen() which cannot be negative. and set the size of ssize_t to 64 bit on windwos 64 bit.

Signed-off-by: Sören Krecker <soekkle@freenet.de>
---
 compat/compiler.h               |  4 ++--
 compat/mingw.c                  | 25 +++++++++++++++----------
 compat/vcbuild/include/unistd.h |  4 ++++
 3 files changed, 21 insertions(+), 12 deletions(-)
Show changes to 3 files +21 −12

compat/compiler.h, compat/mingw.c, compat/vcbuild/include/unistd.h

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..0ff550cef3 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);
+	size_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;
+	size_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; /* read() can return negative values */
+	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,8 @@ 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 %"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 +1985,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);
 	xwcstoutf(value, w_value, len_value);
 
 	/*
@@ -2001,7 +2004,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 +2014,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 +3089,8 @@ static void maybe_redirect_std_handles(void)
  */
 int wmain(int argc, const wchar_t **wargv)
 {
-	int i, maxlen, exit_status;
+	int i, exit_status;
+	size_t maxlen;
 	char *buffer, **save;
 	const char **argv;
 
diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h
index 3a959d124c..a261a925b7 100644
--- a/compat/vcbuild/include/unistd.h
+++ b/compat/vcbuild/include/unistd.h
@@ -14,7 +14,11 @@ typedef _mode_t	mode_t;
 
 #ifndef _SSIZE_T_
 #define _SSIZE_T_
+#ifdef _WIN64
+typedef __int64 _ssize_t;
+#else
 typedef long _ssize_t;
+#endif /* _WIN64 */
 
 #ifndef	_OFF_T_
 #define	_OFF_T_

base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2
-- 
2.39.5
Torsten Bögershausen· Oct 10, 2024, 19:19 UTC · re: Sören Krecker · lore

Re: [PATCH] [PATCH] mimgw: Remove Compiler Warnings

On Thu, Oct 10, 2024 at 12:29:50PM +0200, Sören Krecker wrote:
Show 6 quoted lines
> Remove some complier warnings from msvc in compat/mingw.c for value truncation from 64 bit to 32 bit integers.
>
> Use size_t instead of int as all of the changed variables hold the result of strlen() or wcslen() which cannot be negative.
> and set the size of ssize_t to 64 bit on windwos 64 bit.
>
> Signed-off-by: Sören Krecker <soekkle@freenet.de>

I think that commit message can be improved a little bit. The headline deserves to be shortened, in order to fit the rest of the commit messages in Git. The non-headlines should stay below 72 characters or so, and it could make sense to explain the problem for the readers that are not as familiar with the problem as you.

Something like this, please treat it as inspiration, I am not a user of msvc.

=========================================== mingw.c: Fix complier warnings for a 64 bit msvc

Compiling compat/mingw.c under a 64 bit version of msvc produces warnings. An "int" is 32 bit, and ssize_t or size_t should be 64 bit long. Prepare compat/vcbuild/include/unistd.h to have a 64 bit type _ssize_t, when _WIN64 is defined and 32 bit otherwise.

Further down in this include file, as before, ssize_t is defined as _ssize_t, if needed.

Use size_t instead of int for all variables that hold the result of strlen() or wcslen() (which cannot be negative).

Use ssize_t to hold the return value of read(). ===========================================

However, looking at the current code:
static const char *parse_interpreter(const char *cmd)
{
	static char buf[100];
	char *p, *opt;
	int n, fd;
	/* don't even try a .exe */
	n = strlen(cmd);
	if (n >= 4 && !strcasecmp(cmd+n-4, ".exe"))
		return NULL;
	fd = open(cmd, O_RDONLY);
	if (fd < 0)
		return NULL;
	n = read(fd, buf, sizeof(buf)-1);

It looks as if 2 variables are better: size_t n; ssize_t i; [] n = strlen(cmd); i = read();

Show 150 quoted lines
>
> ---
>  compat/compiler.h               |  4 ++--
>  compat/mingw.c                  | 25 +++++++++++++++----------
>  compat/vcbuild/include/unistd.h |  4 ++++
>  3 files changed, 21 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..0ff550cef3 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);
> +	size_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;
> +	size_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; /* read() can return negative values */
> +	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,8 @@ 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 %"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 +1985,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);
>  	xwcstoutf(value, w_value, len_value);
>
>  	/*
> @@ -2001,7 +2004,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 +2014,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 +3089,8 @@ static void maybe_redirect_std_handles(void)
>   */
>  int wmain(int argc, const wchar_t **wargv)
>  {
> -	int i, maxlen, exit_status;
> +	int i, exit_status;
> +	size_t maxlen;
>  	char *buffer, **save;
>  	const char **argv;
>
> diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h
> index 3a959d124c..a261a925b7 100644
> --- a/compat/vcbuild/include/unistd.h
> +++ b/compat/vcbuild/include/unistd.h
> @@ -14,7 +14,11 @@ typedef _mode_t	mode_t;
>
>  #ifndef _SSIZE_T_
>  #define _SSIZE_T_
> +#ifdef _WIN64
> +typedef __int64 _ssize_t;
> +#else
>  typedef long _ssize_t;
> +#endif /* _WIN64 */
>
>  #ifndef	_OFF_T_
>  #define	_OFF_T_
>
> base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2
> --
> 2.39.5
>
>
Sören Krecker· Oct 12, 2024, 06:22 UTC · re: Torsten Bögershausen · lore

[PATCH v4] mingw.c: Fix complier warnings for a 64 bit msvc

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

Compiling compat/mingw.c under a 64 bit version of msvc produces warnings. An "int" is 32 bit, and ssize_t or size_t should be 64 bit long. Prepare compat/vcbuild/include/unistd.h to have a 64 bit type _ssize_t, when _WIN64 is defined and 32 bit otherwise.

Further down in this include file, as before,ssize_t is defined as _ssize_t, if needed.

Use size_t instead of int for all variables that hold the result of strlen() or wcslen() (which cannot be negative).

Use ssize_t to hold the return value of read().
Signed-off-by: Sören Krecker <soekkle@freenet.de>
---
 compat/compiler.h               |  4 ++--
 compat/mingw.c                  | 25 +++++++++++++++----------
 compat/vcbuild/include/unistd.h |  4 ++++
 3 files changed, 21 insertions(+), 12 deletions(-)
Show changes to 3 files +21 −12

compat/compiler.h, compat/mingw.c, compat/vcbuild/include/unistd.h

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..0ff550cef3 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);
+	size_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;
+	size_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; /* read() can return negative values */
+	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,8 @@ 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 %"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 +1985,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);
 	xwcstoutf(value, w_value, len_value);
 
 	/*
@@ -2001,7 +2004,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 +2014,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 +3089,8 @@ static void maybe_redirect_std_handles(void)
  */
 int wmain(int argc, const wchar_t **wargv)
 {
-	int i, maxlen, exit_status;
+	int i, exit_status;
+	size_t maxlen;
 	char *buffer, **save;
 	const char **argv;
 
diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h
index 3a959d124c..a261a925b7 100644
--- a/compat/vcbuild/include/unistd.h
+++ b/compat/vcbuild/include/unistd.h
@@ -14,7 +14,11 @@ typedef _mode_t	mode_t;
 
 #ifndef _SSIZE_T_
 #define _SSIZE_T_
+#ifdef _WIN64
+typedef __int64 _ssize_t;
+#else
 typedef long _ssize_t;
+#endif /* _WIN64 */
 
 #ifndef	_OFF_T_
 #define	_OFF_T_

base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2
-- 
2.39.5
Torsten Bögershausen· Oct 16, 2024, 16:51 UTC · re: Sören Krecker · lore

Re: [PATCH v4] mingw.c: Fix complier warnings for a 64 bit msvc

On Sat, Oct 12, 2024 at 08:22:43AM +0200, Sören Krecker wrote:
> Remove some complier warnings from msvc in compat/mingw.c for value
> truncation from 64 bit to 32 bit integers.
This looks good to me.
Taylor Blau· Oct 16, 2024, 20:22 UTC · re: Sören Krecker · lore

Re: [PATCH v4] mingw.c: Fix complier warnings for a 64 bit msvc

On Sat, Oct 12, 2024 at 08:22:43AM +0200, Sören Krecker wrote:
Show 10 quoted lines
> Remove some complier warnings from msvc in compat/mingw.c for value
> truncation from 64 bit to 32 bit integers.
>
> Compiling compat/mingw.c under a 64 bit version of msvc produces
> warnings. An "int" is 32 bit, and ssize_t or size_t should be 64 bit
> long. Prepare compat/vcbuild/include/unistd.h to have a 64 bit type
> _ssize_t, when _WIN64 is defined and 32 bit otherwise.
>
> Further down in this include file, as before,ssize_t is defined as
> _ssize_t, if needed.

There is a missing ' ' space character between "before," and "ssize_t", but I fixed it up when queueing.

Thanks!

Thanks, Taylor

Sören Krecker· Oct 17, 2024, 17:18 UTC · re: Taylor Blau · lore

[PATCH v5 0/1] mingw.c: Fix complier warnings for a 64 bit msvc

Hi everyone,
I fix the missing space in the commit message.
Best regards,
Sören
Sören Krecker (1):
  mingw.c: Fix complier warnings for a 64 bit msvc
 compat/compiler.h               |  4 ++--
 compat/mingw.c                  | 25 +++++++++++++++----------
 compat/vcbuild/include/unistd.h |  4 ++++
 3 files changed, 21 insertions(+), 12 deletions(-)
base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2
-- 
2.39.5
Sören Krecker· Oct 17, 2024, 17:18 UTC · re: Sören Krecker · lore

[PATCH 1/1] [PATCH] mingw.c: Fix complier warnings for a 64 bit msvc

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

Compiling compat/mingw.c under a 64 bit version of msvc produces warnings. An "int" is 32 bit, and ssize_t or size_t should be 64 bit long. Prepare compat/vcbuild/include/unistd.h to have a 64 bit type _ssize_t, when _WIN64 is defined and 32 bit otherwise.

Further down in this include file, as before, ssize_t is defined as _ssize_t, if needed.

Use size_t instead of int for all variables that hold the result of strlen() or wcslen() (which cannot be negative).

Use ssize_t to hold the return value of read().
Signed-off-by: Sören Krecker <soekkle@freenet.de>
---
 compat/compiler.h               |  4 ++--
 compat/mingw.c                  | 25 +++++++++++++++----------
 compat/vcbuild/include/unistd.h |  4 ++++
 3 files changed, 21 insertions(+), 12 deletions(-)
Show changes to 3 files +21 −12

compat/compiler.h, compat/mingw.c, compat/vcbuild/include/unistd.h

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..0ff550cef3 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);
+	size_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;
+	size_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; /* read() can return negative values */
+	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,8 @@ 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 %"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 +1985,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);
 	xwcstoutf(value, w_value, len_value);
 
 	/*
@@ -2001,7 +2004,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 +2014,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 +3089,8 @@ static void maybe_redirect_std_handles(void)
  */
 int wmain(int argc, const wchar_t **wargv)
 {
-	int i, maxlen, exit_status;
+	int i, exit_status;
+	size_t maxlen;
 	char *buffer, **save;
 	const char **argv;
 
diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h
index 3a959d124c..a261a925b7 100644
--- a/compat/vcbuild/include/unistd.h
+++ b/compat/vcbuild/include/unistd.h
@@ -14,7 +14,11 @@ typedef _mode_t	mode_t;
 
 #ifndef _SSIZE_T_
 #define _SSIZE_T_
+#ifdef _WIN64
+typedef __int64 _ssize_t;
+#else
 typedef long _ssize_t;
+#endif /* _WIN64 */
 
 #ifndef	_OFF_T_
 #define	_OFF_T_
-- 
2.39.5
Taylor Blau· Oct 17, 2024, 18:43 UTC · re: Sören Krecker · lore

Re: [PATCH 1/1] [PATCH] mingw.c: Fix complier warnings for a 64 bit msvc

On Thu, Oct 17, 2024 at 07:18:20PM +0200, Sören Krecker wrote:
Show 5 quoted lines
> ---
>  compat/compiler.h               |  4 ++--
>  compat/mingw.c                  | 25 +++++++++++++++----------
>  compat/vcbuild/include/unistd.h |  4 ++++
>  3 files changed, 21 insertions(+), 12 deletions(-)

Thanks, this version looks good to me. Unless we hear otherwise, let's start merging this one down.

Thanks, Taylor

Junio C Hamano· Oct 10, 2024, 16:08 UTC · re: Phillip Wood · lore

Re: [PATCH v2 1/1] [PATCH] mimgw: remove Compiler Warnings

Phillip Wood <phillip.wood123@gmail.com> writes:
> Thanks for re-rolling, I think "Fix some compiler warnings" would be
> clearer than "Remove", also "integers" is misspelt.

Though "Fix" is a word with less information than other words we could use. The changes in the patch are primarily about mismatched type, so perhaps

    mingw: use size_t insead of int for lengths
      
would make a better commit title.
I agree with everything you said including this part:
Show 13 quoted lines
> It would also be helpful to explain in the commit message the changes
> to _ssize_t
>
>> +#ifdef _WIN64
>> +typedef __int64 _ssize_t;
>> +#else
>>   typedef long _ssize_t;
>> +#endif // _AMD64
>
> Please note that we do not use "//" comments so this should be "/*
> _WIN64 */" so that the comment matches the opening #ifdef
>
> Thanks for working on this
Thanks.

← back to recent threads