Hi,
> Some of these changes are of dubious value as the string lengths involved
> are guaranteed to be small and there is no scope for overflow. And casting
> only stops the compiler warning, not potential overflow, if any..
Exactly. Where there's no scope for an overflow and compiler is too dumb to
notice that, why not "teaching" it a bit?
> As for the offending mixed int/size_t arithmetic, a better option is to just
> to
> avoid the arithmetic -- code gets clearer and shorter too:
Though, I see no problem with size_t/ptrdiff_t arithmetic when implemented
properly. O:-)
> const char *p = _tcsrchr(find->cFileName, TEXT('.'));
> return p && p != find->cFileName && !_tcsicmp(p+1, ext);
>
> No _tcslen() and no offending size variables.
> And that convoluted original is then gone.
The resulting code would look a lot nicer indeed. Stay tuned for v2 of this
patch.
> - WritePipeAsync(pipe, buf, wcslen(buf) * 2, count, events);
> + WritePipeAsync(pipe, buf, (DWORD)(wcslen(buf) * 2), count, events);
>
> Isn't this identical to the original automatic conversion from size_t to
> DWORD?
> An overly paranoid assert(_countof(buf) < MAXDWORD/2) may catch an
> extremely
> unlikely future coding error, but in what way a cast safer?
I didn't say it is safer. The buf is WCHAR[33] on the stack. There is a safety
zero termination a line before the WritePipeAsync(). Therefore, wcslen(buf)
will always return between 0 and 32. Times two (if I was coding this, I'd use
*sizeof(WCHAR) instead of 2, to make a clear statement, what is multiplication
by 2 about), making it between 0 and 64. Unfortunately, result is size_t where
Win32 API expects a DWORD.
This will always produce a false compiler warning. I muted it by an explicit
cast, so we can focus on more important warnings.
> - WritePipeAsync(pipe, result, wcslen(result) * 2, count, events);
> + WritePipeAsync(pipe, result, (DWORD)(wcslen(result) * 2), count,
> events);
>
> Same here.
The `result` is a Windows error message. Do you really expect Windows error
messages will ever grow beyond 2G?
(2G TCHARs*sizeof(TCHAR) = 4GB > MAXDWORD)
> - WriteFile(stdin_write, input, strlen(input), &written, NULL);
> + WriteFile(stdin_write, input, (DWORD)strlen(input), &written,
> NULL);
>
> As above..
Again, you should observe the broader context of the code. The `input` buffer
is allocated on the heap and its size was calculated as DWORD. (Although, we
could research here, how the size calculation behaves for strings longer than
4G.) Anyway, its size will always be <=MAXDWORD. According to the given
parameters, the WideCharToMultiByte() MUST zero-terminate the `input` buffer.
Therefore, strlen(input) is guaranteed to be <MAXDWORD. Therefore, casting from
size_t to DWORD is perfectly safe.
Best regards,
Simon
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
