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@xxxxxxxxxx>
---
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
