[Spice-devel] [PATCH spice-streaming-agent v2 2/2] explicit instead of static registration for built-in plugins
Christophe de Dinechin
cdupontd at redhat.com
Mon Feb 5 15:35:51 UTC 2018
> On 5 Feb 2018, at 15:29, Lukáš Hrázký <lhrazky at redhat.com> wrote:
>
> The static registration (that is, having a static list of pointers to
> static plugin variables and calling a generic function in the main
> initialization code path to register them) allows to add plugins without
> registering each of them explicitly.
>
> However, in a single codebase, and having very few plugins, there is
> very little advantage to it and the tradeoff for the complexity of this
> initialization is not worth it. A single call for every built-in plugin
> is much more simple and clear.
>
> Signed-off-by: Lukáš Hrázký <lhrazky at redhat.com>
> ---
> src/Makefile.am | 2 --
> src/concrete-agent.cpp | 3 ---
> src/mjpeg-fallback.cpp | 6 +-----
> src/mjpeg-fallback.hpp | 1 +
> src/spice-streaming-agent.cpp | 4 ++++
> src/static-plugin.cpp | 23 -----------------------
> src/static-plugin.hpp | 35 -----------------------------------
> 7 files changed, 6 insertions(+), 68 deletions(-)
> delete mode 100644 src/static-plugin.cpp
> delete mode 100644 src/static-plugin.hpp
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 8d5c5bd..590db1f 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -49,8 +49,6 @@ spice_streaming_agent_LDADD = \
>
> spice_streaming_agent_SOURCES = \
> spice-streaming-agent.cpp \
> - static-plugin.cpp \
> - static-plugin.hpp \
> concrete-agent.cpp \
> concrete-agent.hpp \
> mjpeg-fallback.cpp \
> diff --git a/src/concrete-agent.cpp b/src/concrete-agent.cpp
> index 873a69e..c69da43 100644
> --- a/src/concrete-agent.cpp
> +++ b/src/concrete-agent.cpp
> @@ -11,7 +11,6 @@
> #include <dlfcn.h>
>
> #include "concrete-agent.hpp"
> -#include "static-plugin.hpp"
>
> using namespace std;
> using namespace SpiceStreamingAgent;
> @@ -58,8 +57,6 @@ void ConcreteAgent::AddOption(const char *name, const char *value)
>
> void ConcreteAgent::LoadPlugins(const string &directory)
> {
> - StaticPlugin::InitAll(*this);
> -
> string pattern = directory + "/*.so";
> glob_t globbuf;
>
> diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp
> index 7c918a7..d7c676d 100644
> --- a/src/mjpeg-fallback.cpp
> +++ b/src/mjpeg-fallback.cpp
> @@ -15,7 +15,6 @@
> #include <syslog.h>
> #include <X11/Xlib.h>
>
> -#include "static-plugin.hpp"
> #include "jpeg.hpp"
>
> using namespace std;
> @@ -191,8 +190,7 @@ SpiceVideoCodecType MjpegPlugin::VideoCodecType() const {
> return SPICE_VIDEO_CODEC_TYPE_MJPEG;
> }
>
> -static bool
> -mjpeg_plugin_init(Agent* agent)
> +bool MjpegPlugin::Register(Agent* agent)
> {
> if (agent->Version() != PluginVersion)
> return false;
> @@ -205,5 +203,3 @@ mjpeg_plugin_init(Agent* agent)
>
> return true;
> }
> -
> -static StaticPlugin mjpeg_plugin(mjpeg_plugin_init);
> diff --git a/src/mjpeg-fallback.hpp b/src/mjpeg-fallback.hpp
> index 8044244..0e9ed6a 100644
> --- a/src/mjpeg-fallback.hpp
> +++ b/src/mjpeg-fallback.hpp
> @@ -25,6 +25,7 @@ public:
> unsigned Rank() override;
> void ParseOptions(const ConfigureOption *options);
> SpiceVideoCodecType VideoCodecType() const;
> + static bool Register(Agent* agent);
> private:
> MjpegSettings settings = { 10, 80 };
> };
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index 94d9d25..8458975 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -34,6 +34,7 @@
>
> #include "hexdump.h"
> #include "concrete-agent.hpp"
> +#include "mjpeg-fallback.hpp"
>
> using namespace std;
> using namespace SpiceStreamingAgent;
> @@ -489,6 +490,9 @@ int main(int argc, char* argv[])
> }
> }
>
> + // register built-in plugins
> + MjpegPlugin::Register(&agent);
> +
> agent.LoadPlugins(PLUGINSDIR);
>
> register_interrupts();
> diff --git a/src/static-plugin.cpp b/src/static-plugin.cpp
> deleted file mode 100644
> index d5feb22..0000000
> --- a/src/static-plugin.cpp
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/* Utility to manage registration of plugins compiled statically
> - *
> - * \copyright
> - * Copyright 2017 Red Hat Inc. All rights reserved.
> - */
> -
> -#ifdef HAVE_CONFIG_H
> -#include <config.h>
> -#endif
> -
> -#include <stdlib.h>
> -#include "static-plugin.hpp"
> -
> -using namespace SpiceStreamingAgent;
> -
> -const StaticPlugin *StaticPlugin::list = nullptr;
> -
> -void StaticPlugin::InitAll(Agent& agent)
> -{
> - for (const StaticPlugin* plugin = list; plugin; plugin = plugin->next) {
> - plugin->init_func(&agent);
> - }
> -}
> diff --git a/src/static-plugin.hpp b/src/static-plugin.hpp
> deleted file mode 100644
> index 5436b41..0000000
> --- a/src/static-plugin.hpp
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -/* Utility to manage registration of plugins compiled statically
> - *
> - * \copyright
> - * Copyright 2017 Red Hat Inc. All rights reserved.
> - */
> -#ifndef SPICE_STREAMING_AGENT_STATIC_PLUGIN_HPP
> -#define SPICE_STREAMING_AGENT_STATIC_PLUGIN_HPP
> -
> -#include <spice-streaming-agent/plugin.hpp>
> -
> -namespace SpiceStreamingAgent {
> -
> -class StaticPlugin final {
> -public:
> - StaticPlugin(PluginInitFunc init_func):
> - next(list),
> - init_func(init_func)
> - {
> - list = this;
> - }
> - static void InitAll(Agent& agent);
> -private:
> - // this should be instantiated statically
> - void *operator new(size_t s);
> - void *operator new[](size_t s);
> -
> - const StaticPlugin *const next;
> - const PluginInitFunc* const init_func;
> -
> - static const StaticPlugin *list;
> -};
> -
> -}
> -
> -#endif // SPICE_STREAMING_AGENT_STATIC_PLUGIN_HPP
Looks indeed simpler to me.
Reviewed-by: Christophe de Dinechin <dinechin at redhat.com>
(I’ll leave the ack to Frediano, feel bad about acking while he’s travelling and presumably can’t easily read the patches ;-)
> --
> 2.16.1
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list