PROTON-1453: properly terminate output string buffer

[kgiusti]

Fixes the occasional segfault when running travis (as encountered by aconway)

[astitcher]

I'm not sure about these changes.
They seem to change the character buffer even though they failed. That seems unexpected to me.

Perhaps the tests that failed are written incorrectly?

[alanconway]

Ship It! Wow, that's an obscure bug found by obscure means.

To reply to andrews comments; the function is supposed to return a string in buffer. IMO returning an empty string on error is reasonable and more robust for sloppy code that doesn't check the error return values. I can't think of any reasonably written code that would depend on the value being unchanged in an error, but I can think of plenty of sloppy code that would pass an uninitiailzed buffer and then try to printf it or somesuch even if the return value was non-0

[astitcher]

@alanconway If the calls are documented to always return a value in the string (even upon failure) I won't complain.

As a matter of API design though, a failing call should not alter anything.

[alanconway]

Disagree. IMO it is good design for all return values of a failing call to be set to known failure indicators provided a) there are such indicators, b) it is not costly to do so, and c) it is unlikely there was valuable data in them before call. Provided the API doc is clear that the value will always be set (to "" in an error) that seems to me more robust than leaving random data lying around.

I agree the tests are poorly written for not checking the return value but I think the more robust behavior leads to less support calls.

[kgiusti]

I agree with Alan on this - but in this specific case proton is actually inconsistent in the handling of this buffer. In the schannel impl the buffer is null terminated regardless of outcome in one logic path. The openssl null terminates the fingerprint, cipher name, but not the protocol name. This patch removes the inconsistency.