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

["Torsten Bögershausen" <tboegi@web.de>]

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