Re: [Spice-devel] [PATCH spice-streaming-agent v2 2/2] explicit instead of static registration for built-in plugins
>
> On Fri, 2018-02-16 at 07:23 -0500, Frediano Ziglio wrote:
> > >
> > > > On 6 Feb 2018, at 10:51, Frediano Ziglio wrote:
> > > >
> > > > > >
> > > > > > On 5 Feb 2018, at 15:29, Lukáš Hrázký 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ý
> > > > > > ---
> > > > > > 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
> > > > > >
> > > > > > #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
> > > > > > #include
> > > > > >
> > > > > > -#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
Re: [Spice-devel] [PATCH spice-streaming-agent v2 2/2] explicit instead of static registration for built-in plugins
On Fri, 2018-02-16 at 07:23 -0500, Frediano Ziglio wrote:
> >
> > > On 6 Feb 2018, at 10:51, Frediano Ziglio wrote:
> > >
> > > > >
> > > > > On 5 Feb 2018, at 15:29, Lukáš Hrázký 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ý
> > > > > ---
> > > > > 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
> > > > >
> > > > > #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
> > > > > #include
> > > > >
> > > > > -#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/
Re: [Spice-devel] [PATCH spice-streaming-agent v2 2/2] explicit instead of static registration for built-in plugins
>
> > On 6 Feb 2018, at 10:51, Frediano Ziglio wrote:
> >
> >>>
> >>> On 5 Feb 2018, at 15:29, Lukáš Hrázký 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ý
> >>> ---
> >>> 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
> >>>
> >>> #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
> >>> #include
> >>>
> >>> -#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..000
> >>> --- 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
> >>> -#endif
> >>> -
> >>> -#include
> >>> -#include "static-plugin.hpp"
> >>> -
> >>> -using namespace SpiceStreamingAgent;
> >>> -
> >>> -const StaticPlugin *StaticPlugin::list = nullptr;
> >>> -
> >>> -void StaticPlugin::InitAll(A
Re: [Spice-devel] [PATCH spice-streaming-agent v2 2/2] explicit instead of static registration for built-in plugins
> On 6 Feb 2018, at 10:51, Frediano Ziglio wrote:
>
>>>
>>> On 5 Feb 2018, at 15:29, Lukáš Hrázký 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ý
>>> ---
>>> 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
>>>
>>> #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
>>> #include
>>>
>>> -#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..000
>>> --- 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
>>> -#endif
>>> -
>>> -#include
>>> -#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..000
>>> --- a/src/stat
Re: [Spice-devel] [PATCH spice-streaming-agent v2 2/2] explicit instead of static registration for built-in plugins
>
> > On 5 Feb 2018, at 15:29, Lukáš Hrázký 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ý
> > ---
> > 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
> >
> > #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
> > #include
> >
> > -#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..000
> > --- 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
> > -#endif
> > -
> > -#include
> > -#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..000
> > --- a/src/static-plugin.hpp
> > +++ /dev/null
> > @@ -1,35
Re: [Spice-devel] [PATCH spice-streaming-agent v2 2/2] explicit instead of static registration for built-in plugins
> On 5 Feb 2018, at 15:29, Lukáš Hrázký 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ý
> ---
> 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
>
> #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
> #include
>
> -#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..000
> --- 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
> -#endif
> -
> -#include
> -#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..000
> --- 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
> -
> -namespace SpiceStreami
[Spice-devel] [PATCH spice-streaming-agent v2 2/2] explicit instead of static registration for built-in plugins
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ý
---
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
#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
#include
-#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..000
--- 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
-#endif
-
-#include
-#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..000
--- 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
-
-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);
-
