Sören Krecker <soekkle@xxxxxxxxxx> 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.
