HomeWildfire Games

Upgrade gloox to the development trunk version on Windows, refs #3004. This…

Description

Upgrade gloox to the development trunk version on Windows, refs #3004. This commit needs a rebuild of the glooxwrapper.

This fixes #4705: the TLS connection now works on Windows, also tested with certificate verification and with TLSv1.2 (so TLSv1.0 can now be disabled server-side).

Ideally we should have waited for the release of version 1.0.25, but the development seems to have mostly stopped upstream.

Tested By: maroder
Differential Revision: https://code.wildfiregames.com/D4910

Event Timeline

I got the following warnings when build with --build-shared-glooxwrapper:

libraries\win32\gloox\include\gloox\clientbase.h(769): warning C4373: 'gloox::ClientBase::handlePing': virtual function overrides 'gloox::PingHandler::handlePing', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers
libraries\win32\gloox\include\gloox\pinghandler.h(61): note: see declaration of 'gloox::PingHandler::handlePing'
libraries\win32\gloox\include\gloox\clientbase.h(769): warning C4373: 'gloox::ClientBase::handlePing': virtual function overrides 'gloox::PingHandler::handlePing', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers (compiling source file ..\..\..\source\lobby\StanzaExtensions.cpp)
libraries\win32\gloox\include\gloox\pinghandler.h(61): note: see declaration of 'gloox::PingHandler::handlePing' (compiling source file ..\..\..\source\lobby\StanzaExtensions.cpp)
libraries\win32\gloox\include\gloox\clientbase.h(769): warning C4373: 'gloox::ClientBase::handlePing': virtual function overrides 'gloox::PingHandler::handlePing', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers (compiling source file ..\..\..\source\lobby\scripting\GlooxScriptConversions.cpp)
libraries\win32\gloox\include\gloox\pinghandler.h(61): note: see declaration of 'gloox::PingHandler::handlePing' (compiling source file ..\..\..\source\lobby\scripting\GlooxScriptConversions.cpp)
libraries\win32\gloox\include\gloox\clientbase.h(769): warning C4373: 'gloox::ClientBase::handlePing': virtual function overrides 'gloox::PingHandler::handlePing', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers (compiling source file ..\..\..\source\lobby\XmppClient.cpp)
libraries\win32\gloox\include\gloox\pinghandler.h(61): note: see declaration of 'gloox::PingHandler::handlePing' (compiling source file ..\..\..\source\lobby\XmppClient.cpp)
Itms added a comment.Jan 27 2023, 11:21 AM

Ah shoot, the fix was in D4910, but it got lost when I copied all the headers again before committing. I am actually hesitating between two fixes:

  1. This is the correct fix, but it should be merged upstream instead of dirty patching our Windows includes:
===================================================================
--- libraries/win32/gloox/include/gloox/clientbase.h	(revision 27498)
+++ libraries/win32/gloox/include/gloox/clientbase.h	(working copy)
@@ -766,7 +766,7 @@
 
 #if !defined( GLOOX_MINIMAL ) || defined( WANT_PING )
       // reimplemented from PingHandler
-      virtual void handlePing( const PingType type, const std::string& body );
+      virtual void handlePing( PingType type, const std::string& body );
 #endif // GLOOX_MINIMAL
 
   protected:
  1. I think the more sensible thing to do would be to silence the warning like we do with other libs:
Index: source/lobby/glooxwrapper/glooxwrapper.h
===================================================================
--- source/lobby/glooxwrapper/glooxwrapper.h	(revision 27498)
+++ source/lobby/glooxwrapper/glooxwrapper.h	(working copy)
@@ -68,6 +68,11 @@
 # define _WINDOWS_
 #endif
 
+#if MSC_VERSION
+# pragma warning(push)
+# pragma warning(disable:4373) // "Y virtual function overrides X, previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers"
+#endif
+
 #include <gloox/client.h>
 #include <gloox/mucroom.h>
 #include <gloox/registration.h>
@@ -77,6 +82,10 @@
 #include <gloox/jinglesessionhandler.h>
 #include <gloox/jinglesessionmanager.h>
 
+#if MSC_VERSION
+# pragma warning(pop)
+#endif
+
 #include <cstring>
 
 // Gloox leaves some #define up, we need to undefine them.

But for some unfathomable reason, this has the side effect of triggering C4251 later in the glooxwrapper, so it also needs

Index: source/lobby/glooxwrapper/glooxwrapper.h
===================================================================
--- source/lobby/glooxwrapper/glooxwrapper.h	(revision 27498)
+++ source/lobby/glooxwrapper/glooxwrapper.h	(working copy)
@@ -567,7 +576,14 @@
 		NONCOPYABLE(Registration);
 	private:
 		gloox::Registration* m_Wrapped;
+#if MSC_VERSION
+# pragma warning(push)
+# pragma warning(disable: 4251) // "C4251 can be ignored if your class is derived from a type in the C++ Standard Library"
+#endif
 		std::list<std::shared_ptr<gloox::RegistrationHandler> > m_RegistrationHandlers;
+#if MSC_VERSION
+# pragma warning(pop)
+#endif
 	public:
 		Registration(Client* parent);
 		~Registration();

so I'm a bit bothered and I'd like opinions. I'd go with the first fix unless you prefer the second one.

In rP27490#60081, @Itms wrote:

so I'm a bit bothered and I'd like opinions. I'd go with the first fix unless you prefer the second one.

I don't mind both of them, but upstream should be done and shouldn't be forgotten.

Itms added a comment.Jan 31 2023, 7:52 PM

I went with the const-correct fix and sent it upstream.