[Spice-devel] [PATCH spice-streaming-agent v2 4/9] Introduce a short class to have RAII on the syslog

[Lukáš Hrázký]

On Wed, 2018-06-27 at 10:21 -0400, Frediano Ziglio wrote:
> > 
> > Signed-off-by: Lukáš Hrázký <lhrazky at redhat.com>
> > ---
> >  src/spice-streaming-agent.cpp | 20 +++++++++++++++++---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> > index be133e4..b434180 100644
> > --- a/src/spice-streaming-agent.cpp
> > +++ b/src/spice-streaming-agent.cpp
> > @@ -386,6 +386,22 @@ do_capture(StreamPort &stream_port, FrameLog &frame_log)
> >      }
> >  }
> >  
> > +class SyslogRAII
> > +{
> > +public:
> > +    SyslogRAII()
> > +    {
> > +        openlog("spice-streaming-agent",
> > +                isatty(fileno(stderr)) ? (LOG_PERROR|LOG_PID) : LOG_PID,
> > LOG_USER);
> > +    }
> > +
> > +    ~SyslogRAII()
> > +    {
> > +        closelog();
> > +    }
> > +};
> > +
> > +
> >  int main(int argc, char* argv[])
> >  {
> >      const char *stream_port_name =
> >      "/dev/virtio-ports/org.spice-space.stream.0";
> > @@ -409,8 +425,7 @@ int main(int argc, char* argv[])
> >      };
> >      std::vector<std::string> old_args(argv, argv+argc);
> >  
> > -    openlog("spice-streaming-agent",
> > -            isatty(fileno(stderr)) ? (LOG_PERROR|LOG_PID) : LOG_PID,
> > LOG_USER);
> > +    SyslogRAII syslog_raii;
> >  
> >      setlogmask(LOG_UPTO(LOG_WARNING));
> >  
> > @@ -501,6 +516,5 @@ int main(int argc, char* argv[])
> >          ret = EXIT_FAILURE;
> >      }
> >  
> > -    closelog();
> >      return ret;
> >  }
> 
> Honestly I don't see this change as that great.

I'm not thrilled about it either... :)

> Perhaps would just be easier to remove closelog call as is not mandatory.

If you want... I'm not sure which is better. While ugly, the class
doesn't hurt much here.

> Is not greatly encapsulated, calling ::syslog is always possible, before or
> after the object is created.
> I don't like mush a class name containing RAII is like saying that RAII
> is not much the default and we need to specify it.

The RAII is in there to denote the purpose of the class, part of that
being the fact it doesn't entirely encapsulate the syslog (I don't
think that's a good idea there).

> Frediano