upnp: add class UpnpCallback

Each Upnp*Async() call passes a new cookie pointer, and the cookie
passed to UpnpRegisterClient() appears to be ignored.  Using this
interface is a more elegant approach than having one single "handler"
function.

Makefile.am

@@ -222,6 +222,7 @@ UPNP_SOURCES = \
 	src/lib/upnp/Discovery.cxx src/lib/upnp/Discovery.hxx \
 	src/lib/upnp/Domain.cxx src/lib/upnp/Domain.hxx \
 	src/lib/upnp/ixmlwrap.cxx src/lib/upnp/ixmlwrap.hxx \
+	src/lib/upnp/Callback.hxx \
 	src/lib/upnp/upnpplib.cxx src/lib/upnp/upnpplib.hxx \
 	src/lib/upnp/Util.cxx src/lib/upnp/Util.hxx \
 	src/lib/upnp/WorkQueue.hxx \

src/lib/upnp/Callback.hxx

@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2003-2014 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef MPD_UPNP_CALLBACK_HXX
+#define MPD_UPNP_CALLBACK_HXX
+
+#include <upnp/upnp.h>
+
+/**
+ * A class that is supposed to be used for libupnp asynchronous
+ * callbacks.
+ */
+class UpnpCallback {
+public:
+	/**
+	 * Pass this value as "cookie" pointer to libupnp asynchronous
+	 * functions.
+	 */
+	void *GetUpnpCookie() {
+		return this;
+	}
+
+	static UpnpCallback &FromUpnpCookie(void *cookie) {
+		return *(UpnpCallback *)cookie;
+	}
+
+	virtual int Invoke(Upnp_EventType et, void *evp) = 0;
+};
+
+#endif

src/lib/upnp/Discovery.cxx

@@ -184,8 +184,8 @@ UPnPDeviceDirectory::OnByeBye(Upnp_Discovery *disco)
 // thread context.
 // Example: ContentDirectories appearing and disappearing from the network
 // We queue a task for our worker thread(s)
-inline int
+int
-UPnPDeviceDirectory::cluCallBack(Upnp_EventType et, void *evp)
+UPnPDeviceDirectory::Invoke(Upnp_EventType et, void *evp)
 {
 	switch (et) {
 	case UPNP_DISCOVERY_SEARCH_RESULT:
@@ -254,10 +254,6 @@ UPnPDeviceDirectory::Start(Error &error)
 		return false;
 	}
 
-	lib->SetHandler([this](Upnp_EventType type, void *event){
-			cluCallBack(type, event);
-		});
-
 	return search(error);
 }
 
@@ -271,7 +267,7 @@ UPnPDeviceDirectory::search(Error &error)
 
 	// We search both for device and service just in case.
 	int code = UpnpSearchAsync(lib->getclh(), m_searchTimeout,
-				   ContentDirectorySType, lib);
+				   ContentDirectorySType, GetUpnpCookie());
 	if (code != UPNP_E_SUCCESS) {
 		error.Format(upnp_domain, code,
 			     "UpnpSearchAsync() failed: %s",
@@ -280,7 +276,7 @@ UPnPDeviceDirectory::search(Error &error)
 	}
 
 	code = UpnpSearchAsync(lib->getclh(), m_searchTimeout,
-			       MediaServerDType, lib);
+			       MediaServerDType, GetUpnpCookie());
 	if (code != UPNP_E_SUCCESS) {
 		error.Format(upnp_domain, code,
 			     "UpnpSearchAsync() failed: %s",

src/lib/upnp/Discovery.hxx

@@ -20,6 +20,7 @@
 #ifndef _UPNPPDISC_H_X_INCLUDED_
 #define _UPNPPDISC_H_X_INCLUDED_
 
+#include "Callback.hxx"
 #include "Device.hxx"
 #include "WorkQueue.hxx"
 #include "thread/Mutex.hxx"
@@ -46,7 +47,7 @@ public:
  * We are only interested in MediaServers with a ContentDirectory service
  * for now, but this could be made more general, by removing the filtering.
  */
-class UPnPDeviceDirectory {
+class UPnPDeviceDirectory final : UpnpCallback {
 	/**
 	 * Each appropriate discovery event (executing in a libupnp thread
 	 * context) queues the following task object for processing by the
@@ -154,6 +155,9 @@ private:
 	int OnAlive(Upnp_Discovery *disco);
 	int OnByeBye(Upnp_Discovery *disco);
 	int cluCallBack(Upnp_EventType et, void *evp);
+
+	/* virtual methods from class UpnpCallback */
+	virtual int Invoke(Upnp_EventType et, void *evp) override;
 };
 
 

src/lib/upnp/upnpplib.cxx

@@ -19,6 +19,7 @@
 
 #include "config.h"
 #include "upnpplib.hxx"
+#include "Callback.hxx"
 #include "Domain.hxx"
 #include "Log.hxx"
 
@@ -37,7 +38,7 @@ LibUPnP::LibUPnP()
 
 	UpnpSetMaxContentLength(2000*1024);
 
-	code = UpnpRegisterClient(o_callback, (void *)this, &m_clh);
+	code = UpnpRegisterClient(o_callback, nullptr, &m_clh);
 	if (code != UPNP_E_SUCCESS) {
 		init_error.Format(upnp_domain, code,
 				  "UpnpRegisterClient() failed: %s",
@@ -52,12 +53,14 @@ LibUPnP::LibUPnP()
 int
 LibUPnP::o_callback(Upnp_EventType et, void* evp, void* cookie)
 {
-	LibUPnP *ulib = (LibUPnP *)cookie;
+	if (cookie == nullptr)
+		/* this is the cookie passed to UpnpRegisterClient();
+		   but can this ever happen?  Will libupnp ever invoke
+		   the registered callback without that cookie? */
+		return UPNP_E_SUCCESS;
 
-	if (ulib->handler)
+	UpnpCallback &callback = UpnpCallback::FromUpnpCookie(cookie);
-		ulib->handler(et, evp);
+	return callback.Invoke(et, evp);
-
-	return UPNP_E_SUCCESS;
 }
 
 LibUPnP::~LibUPnP()

src/lib/upnp/upnpplib.hxx

@@ -24,17 +24,11 @@
 
 #include <upnp/upnp.h>
 
-#include <functional>
-
 /** Our link to libupnp. Initialize and keep the handle around */
 class LibUPnP {
-	typedef std::function<void(Upnp_EventType type, void *event)> Handler;
-
 	Error init_error;
 	UpnpClient_Handle m_clh;
 
-	Handler handler;
-
 	static int o_callback(Upnp_EventType, void *, void *);
 
 public:
@@ -56,11 +50,6 @@ public:
 		return init_error;
 	}
 
-	template<typename T>
-	void SetHandler(T &&_handler) {
-		handler = std::forward<T>(_handler);
-	}
-
 	UpnpClient_Handle getclh()
 	{
 		return m_clh;