Forum OpenACS Development: Re: NaviServer on Windows: [ns_info threads] returning unbalanced braces

Dear Gustaf,
thank you very much for having replicated the issue.
I'm not sure I agree completely with your explanation.
If you're explanation was correct how come the following small modification (which I believe is the memory bug I was referring too) in the file dstring.c,
// #ifdef _WIN32
// while (result == -1 && errno == ERANGE) {
// newLength = dsPtr->spaceAvl * 2;
//#else
if ((size_t)result >= bufLength) {
newLength = dsPtr->spaceAvl + (result - (int)bufLength);
//#endif
makes:
1. the problem with the unbalanced braces disappear
2. the odd characters in the log disappear
3. the erratic behaviour disappear?

Thank you,
Maurizio

maurizio, does this mean, you see with this change still the "unbalanced braces" issue? If so, make sure, that in your local changes, you do not alter the definitions of naviserver in respect to snprintf/vsnprintf (as it was in the case with the changes you sent to me around April 2016).

Background: the committed change (not sure, if the snippet you posted is the same) affects the dstring infrastructure, which is all over the code to append to a string. A buffer overflow on such a string can have many weird effects. Up to Visual Studio 2015, the functions vsnprintf() and _vsnprintf() behaved in the same, microsoft specific way, namely returning -1 in case, the provided buffer was too small, not guaranteeing null termination.

With the introduction of Visual Studio 2015, the behavior of vsnprintf() changed to become c99 compliant, but __vsnprintf() stayed the same. the c99 standard states, that in cases where the buffer is too small, the function should return the number of bytes that would have been written. That is a very different behavior for recognizing buffer grow requests and buffer size management. The same change of behavior also affects e.g. snprintf()/_snprintf(). So, when using these functions, a change from a Visual Studio before version 2015 to a newer version requires code changes. Convince yourself by compiling with an earlier version of Visual Studio!

So far, i see no obvious connection between the dstring function and the WSASend() problem. So far, i could not reproduce this issue. I tried yesterday multiple times with requests for a 6+ MB file, but everything worked fine. I find it hard to believe, that the specification

if both lpOverlapped and lpCompletionRoutine are NULL, the socket in this function will be treated as a non-overlapped socket. (https://msdn.microsoft.com/en-us/library/windows/desktop/ms742203.aspx)

is not valid in newer versions.

If someone has instructions, how to reproduce the issue reliably, i would appreciate it. i can't test maurizio's binaries in my setup, since i don't have 10GB free on the c: partition, and i failed to extend the partition in the vm under mac os x; so i need probably a different, completely new setup, for which i have no time now).

Do i read your mail correctly that now all know issues are gone? That would be great.

Concerning the change around WSAsend(): The difference is not easy to spot but due to a type conversion, since n1 (type DWORD) was originally assigned -1 in error conditions. Since DWORD is defined as unsigned 32bit quantity, bad things happen when its value is assigned to a type of a different size and signedness (here: ssize_t; probably not an issue in 32bit versions). Interestingly, this type issue is not flagged by sonarsrv: the current version of nssock.c in sonarsrv is still the "old" code and shows 0 issues [1]. I've cleaned this code yesterday to prepare for debugging return-codes from WSASend().

Concerning send(): When send() is used as shown in your snippet, the code will produce incorrect results when nbufs <> 1. The reason for using WSAsend() instead of send() is that the former supports the more efficient scatter/gather interface.

all the best
-gn

[1] http://sonarsrv.spazioit.com/code/index?id=my%3Anaviserver#/my%3Anaviserver%3Ansd%2Fsock.c