* [PATCH] mimgw: remove Compiler Warnings @ 2024-10-09 10:35 Sören Krecker 2024-10-09 15:26 ` Torsten Bögershausen 2024-10-09 16:13 ` Phillip Wood 0 siblings, 2 replies; 16+ messages in thread From: Sören Krecker @ 2024-10-09 10:35 UTC (permalink / raw) To: git; +Cc: Sören Krecker 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(-) 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] mimgw: remove Compiler Warnings 2024-10-09 10:35 [PATCH] mimgw: remove Compiler Warnings Sören Krecker @ 2024-10-09 15:26 ` Torsten Bögershausen 2024-10-09 16:13 ` Phillip Wood 1 sibling, 0 replies; 16+ messages in thread From: Torsten Bögershausen @ 2024-10-09 15:26 UTC (permalink / raw) To: Sören Krecker; +Cc: git 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 > > 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 ? > + 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 ? > 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 Is this needed, or is it a rest from a trial to use _ssize_t instead of ssize_t ? > + > + > > #ifndef _OFF_T_ > #define _OFF_T_ > > base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2 > -- > 2.39.5 > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mimgw: remove Compiler Warnings 2024-10-09 10:35 [PATCH] mimgw: remove Compiler Warnings Sören Krecker 2024-10-09 15:26 ` Torsten Bögershausen @ 2024-10-09 16:13 ` Phillip Wood 2024-10-09 17:13 ` Sören Krecker 1 sibling, 1 reply; 16+ messages in thread From: Phillip Wood @ 2024-10-09 16:13 UTC (permalink / raw) To: Sören Krecker, git; +Cc: Torsten Bögershausen 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. > 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. > 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 > > 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 > @@ -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 > @@ -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 > 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 > @@ -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 > @@ -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 > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] mimgw: remove Compiler Warnings 2024-10-09 16:13 ` Phillip Wood @ 2024-10-09 17:13 ` Sören Krecker 2024-10-09 17:13 ` [PATCH v2 1/1] " Sören Krecker 0 siblings, 1 reply; 16+ messages in thread From: Sören Krecker @ 2024-10-09 17:13 UTC (permalink / raw) To: git; +Cc: tboegi, phillip.wood, Sören Krecker 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/1] [PATCH] mimgw: remove Compiler Warnings 2024-10-09 17:13 ` Sören Krecker @ 2024-10-09 17:13 ` Sören Krecker 2024-10-09 18:20 ` Junio C Hamano 2024-10-10 8:59 ` Phillip Wood 0 siblings, 2 replies; 16+ messages in thread From: Sören Krecker @ 2024-10-09 17:13 UTC (permalink / raw) To: git; +Cc: tboegi, phillip.wood, Sören Krecker 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(-) 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/1] [PATCH] mimgw: remove Compiler Warnings 2024-10-09 17:13 ` [PATCH v2 1/1] " Sören Krecker @ 2024-10-09 18:20 ` Junio C Hamano 2024-10-10 8:59 ` Phillip Wood 1 sibling, 0 replies; 16+ messages in thread From: Junio C Hamano @ 2024-10-09 18:20 UTC (permalink / raw) To: Sören Krecker; +Cc: git, tboegi, phillip.wood 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? > 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. > @@ -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. > @@ -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); > @@ -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. > @@ -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. > @@ -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. > 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/1] [PATCH] mimgw: remove Compiler Warnings 2024-10-09 17:13 ` [PATCH v2 1/1] " Sören Krecker 2024-10-09 18:20 ` Junio C Hamano @ 2024-10-10 8:59 ` Phillip Wood 2024-10-10 10:29 ` [PATCH] [PATCH] mimgw: Remove " Sören Krecker 2024-10-10 16:08 ` [PATCH v2 1/1] [PATCH] mimgw: remove Compiler Warnings Junio C Hamano 1 sibling, 2 replies; 16+ messages in thread From: Phillip Wood @ 2024-10-10 8:59 UTC (permalink / raw) To: Sören Krecker, git; +Cc: tboegi, phillip.wood, Junio C Hamano 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 > +#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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] [PATCH] mimgw: Remove Compiler Warnings 2024-10-10 8:59 ` Phillip Wood @ 2024-10-10 10:29 ` Sören Krecker 2024-10-10 19:19 ` Torsten Bögershausen 2024-10-10 16:08 ` [PATCH v2 1/1] [PATCH] mimgw: remove Compiler Warnings Junio C Hamano 1 sibling, 1 reply; 16+ messages in thread From: Sören Krecker @ 2024-10-10 10:29 UTC (permalink / raw) To: git; +Cc: tboegi, phillip.wood, gitster, Sören Krecker 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(-) 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] [PATCH] mimgw: Remove Compiler Warnings 2024-10-10 10:29 ` [PATCH] [PATCH] mimgw: Remove " Sören Krecker @ 2024-10-10 19:19 ` Torsten Bögershausen 2024-10-12 6:22 ` [PATCH v4] mingw.c: Fix complier warnings for a 64 bit msvc Sören Krecker 0 siblings, 1 reply; 16+ messages in thread From: Torsten Bögershausen @ 2024-10-10 19:19 UTC (permalink / raw) To: Sören Krecker; +Cc: git, phillip.wood, gitster On Thu, Oct 10, 2024 at 12:29:50PM +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. > > 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(); > > --- > 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 > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4] mingw.c: Fix complier warnings for a 64 bit msvc 2024-10-10 19:19 ` Torsten Bögershausen @ 2024-10-12 6:22 ` Sören Krecker 2024-10-16 16:51 ` Torsten Bögershausen 2024-10-16 20:22 ` Taylor Blau 0 siblings, 2 replies; 16+ messages in thread From: Sören Krecker @ 2024-10-12 6:22 UTC (permalink / raw) To: git; +Cc: tboegi, phillip.wood, gitster, Sören Krecker 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(-) 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v4] mingw.c: Fix complier warnings for a 64 bit msvc 2024-10-12 6:22 ` [PATCH v4] mingw.c: Fix complier warnings for a 64 bit msvc Sören Krecker @ 2024-10-16 16:51 ` Torsten Bögershausen 2024-10-16 20:22 ` Taylor Blau 1 sibling, 0 replies; 16+ messages in thread From: Torsten Bögershausen @ 2024-10-16 16:51 UTC (permalink / raw) To: Sören Krecker; +Cc: git, phillip.wood, gitster 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] mingw.c: Fix complier warnings for a 64 bit msvc 2024-10-12 6:22 ` [PATCH v4] mingw.c: Fix complier warnings for a 64 bit msvc Sören Krecker 2024-10-16 16:51 ` Torsten Bögershausen @ 2024-10-16 20:22 ` Taylor Blau 2024-10-17 17:18 ` [PATCH v5 0/1] " Sören Krecker 1 sibling, 1 reply; 16+ messages in thread From: Taylor Blau @ 2024-10-16 20:22 UTC (permalink / raw) To: Sören Krecker; +Cc: git, tboegi, phillip.wood, gitster 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. > > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v5 0/1] mingw.c: Fix complier warnings for a 64 bit msvc 2024-10-16 20:22 ` Taylor Blau @ 2024-10-17 17:18 ` Sören Krecker 2024-10-17 17:18 ` [PATCH 1/1] [PATCH] " Sören Krecker 0 siblings, 1 reply; 16+ messages in thread From: Sören Krecker @ 2024-10-17 17:18 UTC (permalink / raw) To: git; +Cc: tboegi, phillip.wood, gitster, me, Sören Krecker 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/1] [PATCH] mingw.c: Fix complier warnings for a 64 bit msvc 2024-10-17 17:18 ` [PATCH v5 0/1] " Sören Krecker @ 2024-10-17 17:18 ` Sören Krecker 2024-10-17 18:43 ` Taylor Blau 0 siblings, 1 reply; 16+ messages in thread From: Sören Krecker @ 2024-10-17 17:18 UTC (permalink / raw) To: git; +Cc: tboegi, phillip.wood, gitster, me, Sören Krecker 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(-) 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/1] [PATCH] mingw.c: Fix complier warnings for a 64 bit msvc 2024-10-17 17:18 ` [PATCH 1/1] [PATCH] " Sören Krecker @ 2024-10-17 18:43 ` Taylor Blau 0 siblings, 0 replies; 16+ messages in thread From: Taylor Blau @ 2024-10-17 18:43 UTC (permalink / raw) To: Sören Krecker; +Cc: git, tboegi, phillip.wood, gitster On Thu, Oct 17, 2024 at 07:18:20PM +0200, Sören Krecker wrote: > --- > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/1] [PATCH] mimgw: remove Compiler Warnings 2024-10-10 8:59 ` Phillip Wood 2024-10-10 10:29 ` [PATCH] [PATCH] mimgw: Remove " Sören Krecker @ 2024-10-10 16:08 ` Junio C Hamano 1 sibling, 0 replies; 16+ messages in thread From: Junio C Hamano @ 2024-10-10 16:08 UTC (permalink / raw) To: Phillip Wood; +Cc: Sören Krecker, git, tboegi, phillip.wood 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: > 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-10-17 18:43 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-09 10:35 [PATCH] mimgw: remove Compiler Warnings Sören Krecker 2024-10-09 15:26 ` Torsten Bögershausen 2024-10-09 16:13 ` Phillip Wood 2024-10-09 17:13 ` Sören Krecker 2024-10-09 17:13 ` [PATCH v2 1/1] " Sören Krecker 2024-10-09 18:20 ` Junio C Hamano 2024-10-10 8:59 ` Phillip Wood 2024-10-10 10:29 ` [PATCH] [PATCH] mimgw: Remove " Sören Krecker 2024-10-10 19:19 ` Torsten Bögershausen 2024-10-12 6:22 ` [PATCH v4] mingw.c: Fix complier warnings for a 64 bit msvc Sören Krecker 2024-10-16 16:51 ` Torsten Bögershausen 2024-10-16 20:22 ` Taylor Blau 2024-10-17 17:18 ` [PATCH v5 0/1] " Sören Krecker 2024-10-17 17:18 ` [PATCH 1/1] [PATCH] " Sören Krecker 2024-10-17 18:43 ` Taylor Blau 2024-10-10 16:08 ` [PATCH v2 1/1] [PATCH] mimgw: remove Compiler Warnings Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).