Pointed out by a clang warning, original patch by leper.
Details
- Reviewers
Stan - Group Reviewers
Windows Developers - Commits
- rP21503: Do not rely on undefined behavior in an unsupported part of the glooxwrapper…
Build and test by building the glooxwrapper (update-workspaces.bat --build-shared-glooxwrapper).
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 4845 Build 8396: Vulcan Build Jenkins Build 8395: arc lint + arc unit
Event Timeline
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/993/ for more details.
This does not build, both on Windows and Linux: linking fails with undefined reference to debug_break() and debug_OnAssertionFailure().
Don't forget you need to call update-workspaces.sh --build-shared-glooxwrapper to actually run a build of it.
Actually fail. Not really more tested than the previous iteration, though I have had this iteration in my local copy for a while.
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... ../../../source/lobby/glooxwrapper/glooxwrapper.cpp:222:48: warning: unused parameter ‘from’ [-Wunused-parameter] virtual void handleDataForm(const gloox::JID& from, const gloox::DataForm& UNUSED(form)) ^ ../../../source/lobby/glooxwrapper/glooxwrapper.cpp:228:43: warning: unused parameter ‘from’ [-Wunused-parameter] virtual void handleOOB(const gloox::JID& from, const gloox::OOB& UNUSED(oob)) ^ Build (debug)... ../../../source/lobby/glooxwrapper/glooxwrapper.cpp:222:48: warning: unused parameter ‘from’ [-Wunused-parameter] virtual void handleDataForm(const gloox::JID& from, const gloox::DataForm& UNUSED(form)) ^ ../../../source/lobby/glooxwrapper/glooxwrapper.cpp:228:43: warning: unused parameter ‘from’ [-Wunused-parameter] virtual void handleOOB(const gloox::JID& from, const gloox::OOB& UNUSED(oob)) ^ Running release tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Tested and works, I accept it but you should add UNUSED to the from parameters as reported by Vulcan.
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Feel free to commit this in case you think it helps with not invoking undefined behaviour.
I was looking at this again and I don't know the glooxwrapper well enough to determine why those two functions are "not supported" (I don't see the difference in the code between them and, for instance, handleRegistrationResult). Once I understand this maybe I will see whether there is a specific reason for failing here instead of just doing nothing in the function (like we do in all the other void functions that are marked as "Not supported"). ?
Don't explicitly fail, but do nothing (as we do for the rest of the unsupported API) instead of relying on
undefined behavior.
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/differential/1/display/redirect
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/differential/2/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/3/display/redirect
Replaced
if not exist ..\workspaces\vc2015\SKIP_PREMAKE_HERE premake5\bin\release\premake5 --outpath="../workspaces/vc2015" --use-shared-glooxwrapper %* vs2015
by
if not exist ..\workspaces\vc2015\SKIP_PREMAKE_HERE premake5\bin\release\premake5 --outpath="../workspaces/vc2015" --build-shared-glooxwrapper %* vs2015
Built , no errors, Then logged into the lobby successfully. Anything else ?