Looks reasonable to me.
Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
On Thu, 2018-09-27 at 14:10 +0100, Frediano Ziglio wrote:
> Usually when references are used ownership is not moved.
> Avoid to use references to confuse code reader.
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> include/spice-streaming-agent/plugin.hpp | 4 +++-
> src/concrete-agent.cpp | 4 ++--
> src/concrete-agent.hpp | 2 +-
> src/gst-plugin.cpp | 4 ++--
> src/mjpeg-fallback.cpp | 4 ++--
> 5 files changed, 10 insertions(+), 8 deletions(-)
>
> Changes since v1:
> - use std::shared_ptr
>
> diff --git a/include/spice-streaming-agent/plugin.hpp
> b/include/spice-streaming-agent/plugin.hpp
> index e9acf2d..3b265d9 100644
> --- a/include/spice-streaming-agent/plugin.hpp
> +++ b/include/spice-streaming-agent/plugin.hpp
> @@ -8,6 +8,7 @@
> #define SPICE_STREAMING_AGENT_PLUGIN_HPP
>
> #include <spice/enums.h>
> +#include <memory>
>
> /*!
> * \file
> @@ -104,8 +105,9 @@ class Agent
> public:
> /*!
> * Register a plugin in the system.
> + * Agent will take ownership of the plugin.
> */
> - virtual void Register(Plugin& plugin) = 0;
> + virtual void Register(const std::shared_ptr<Plugin>& plugin) =
> 0;
>
> /*!
> * Get options array.
> diff --git a/src/concrete-agent.cpp b/src/concrete-agent.cpp
> index 1f8b4b4..f94aead 100644
> --- a/src/concrete-agent.cpp
> +++ b/src/concrete-agent.cpp
> @@ -37,9 +37,9 @@ bool
> ConcreteAgent::PluginVersionIsCompatible(unsigned pluginVersion)
> const
> MinorVersion(version) >= MinorVersion(pluginVersion);
> }
>
> -void ConcreteAgent::Register(Plugin& plugin)
> +void ConcreteAgent::Register(const std::shared_ptr<Plugin>& plugin)
> {
> - plugins.push_back(std::shared_ptr<Plugin>(&plugin));
> + plugins.push_back(plugin);
> }
>
> const ConfigureOption* ConcreteAgent::Options() const
> diff --git a/src/concrete-agent.hpp b/src/concrete-agent.hpp
> index c0cf9ba..99dcf54 100644
> --- a/src/concrete-agent.hpp
> +++ b/src/concrete-agent.hpp
> @@ -27,7 +27,7 @@ class ConcreteAgent final : public Agent
> {
> public:
> ConcreteAgent();
> - void Register(Plugin& plugin) override;
> + void Register(const std::shared_ptr<Plugin>& plugin) override;
> const ConfigureOption* Options() const override;
> void LoadPlugins(const std::string &directory);
> // pointer must remain valid
> diff --git a/src/gst-plugin.cpp b/src/gst-plugin.cpp
> index 6eb9e7e..097ef9d 100644
> --- a/src/gst-plugin.cpp
> +++ b/src/gst-plugin.cpp
> @@ -452,11 +452,11 @@ SPICE_STREAMING_AGENT_PLUGIN(agent)
> {
> gst_init(nullptr, nullptr);
>
> - std::unique_ptr<GstreamerPlugin> plugin(new GstreamerPlugin());
> + auto plugin = std::make_shared<GstreamerPlugin>();
>
> plugin->ParseOptions(agent->Options());
>
> - agent->Register(*plugin.release());
> + agent->Register(plugin);
>
> return true;
> }
> diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp
> index 08353f2..8081007 100644
> --- a/src/mjpeg-fallback.cpp
> +++ b/src/mjpeg-fallback.cpp
> @@ -180,7 +180,7 @@ SpiceVideoCodecType MjpegPlugin::VideoCodecType()
> const {
>
> bool MjpegPlugin::Register(Agent* agent)
> {
> - std::unique_ptr<MjpegPlugin> plugin(new MjpegPlugin());
> + auto plugin = std::make_shared<MjpegPlugin>();
>
> try {
> plugin->ParseOptions(agent->Options());
> @@ -188,7 +188,7 @@ bool MjpegPlugin::Register(Agent* agent)
> syslog(LOG_ERR, "Error parsing plugin option: %s",
> e.what());
> }
>
> - agent->Register(*plugin.release());
> + agent->Register(plugin);
>
> return true;
> }
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel