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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2018-06-27 at 10:21 -0400, Frediano Ziglio wrote:
> > 
> > Signed-off-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
> > ---
> >  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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]