[Spice-devel] [spice-streaming-agent PATCH 2/2] LoadPlugin: call dlclose upon failure

[Jonathon Jongsma]

On Wed, 2018-08-01 at 19:04 +0300, Uri Lublin wrote:
> On 08/01/2018 03:43 PM, Uri Lublin wrote:
> > Signed-off-by: Uri Lublin <uril at redhat.com>
> > ---
> >   src/concrete-agent.cpp | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/src/concrete-agent.cpp b/src/concrete-agent.cpp
> > index ca666d7..f26b23c 100644
> > --- a/src/concrete-agent.cpp
> > +++ b/src/concrete-agent.cpp
> > @@ -80,6 +80,7 @@ void ConcreteAgent::LoadPlugin(const std::string
> > &plugin_filename)
> >       if (!dl) {
> >           syslog(LOG_ERR, "error loading plugin %s: %s",
> >                  plugin_filename.c_str(), dlerror());
> > +        dlclose(dl);
> 
> I'll remove this part.
> If dl is NULL (dlopen failed) no need to dlclose it.
> 
> >           return;
> >       }
> >   
> > @@ -88,6 +89,7 @@ void ConcreteAgent::LoadPlugin(const std::string
> > &plugin_filename)
> >       if (!version) {
> >           syslog(LOG_ERR, "error loading plugin %s: no version
> > information",
> >                  plugin_filename.c_str());
> > +        dlclose(dl);
> >           return;
> >       }
> >       if (!PluginVersionIsCompatible(*version)) {
> > @@ -95,6 +97,7 @@ void ConcreteAgent::LoadPlugin(const std::string
> > &plugin_filename)
> >                  "error loading plugin %s: plugin interface version
> > %u.%u not accepted",
> >                  plugin_filename.c_str(),
> >                  MajorVersion(*version), MinorVersion(*version));
> > +        dlclose(dl);
> >           return;
> >       }
> >   
> > 
> 

Looks OK. Another option would simply be to wrap the whole function in
the try block and throw a std::runtime_error since we are already
calling dlclose in the existing exception handler.

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>