rP8925 missed to initialize the m_ErrorBuffer with an empty string.
This leads to undefined data being passed to JS and displayed in the status caption.
Also curl_easy_strerror should be used if the error buffer wasn't written.
Details
Shutdown your network interface, start the UserReporter.
You see a different caption with random data each time you start the unpatched code and a proper string with the patch.
Notice that curl_easy_strerror provided useful error strings here, because the error buffer was not written to.
https://curl.haxx.se/libcurl/c/CURLOPT_ERRORBUFFER.html
Notice that we probably don't want to translate 93 cURL error strings:
https://curl.haxx.se/libcurl/c/libcurl-errors.html#CURLEOK
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/732/display/redirect
source/ps/UserReport.cpp | ||
---|---|---|
350 ↗ | (On Diff #6884) | Couldn't be m_ErrorBuffer = {0} |
source/ps/UserReport.cpp | ||
---|---|---|
350 ↗ | (On Diff #6884) |
http://giyf.com
I rather display no error than "no error". Also it would lead to uglier code because the special string has to be tested for the in the condition added below. |
I failed to figure out whether the string concatenation with unintialized data or the return value of the JS Interface function constitutes undefined behavior or whether that is just defined behavior returning indeterminate data.
http://www.cplusplus.com/reference/string/string/operator+/ says
If s is not a null-terminated character sequence, it causes undefined behavior.
But it doesn't say what s is. Also any data maybe considered a null-terminated character sequence.
Either way broken.
ps/trunk/source/ps/UserReport.cpp | ||
---|---|---|
360 | Not related to this revision proposal, but is it a good idea to not use a curl_multi for this? If the report get sufficently large, this could cause issues, no? |
ps/trunk/source/ps/UserReport.cpp | ||
---|---|---|
360 | The UserReporter is running in a separate thread, so it's not a problem to use the synchroneous / blocking curl easy handlers. |
ps/trunk/source/ps/UserReport.cpp | ||
---|---|---|
360 |
Ah, I see. |