On Tue, Sep 04, 2018 at 02:44:29PM +0200, Jakub Janku wrote:
> > > > > + if (err) {
> > > > > + g_printerr("Invalid arguments, %s\n", err->message);
> > > >
> > > > We don't use g_printerr() or any g_log() in this code
> > > > yet.
> > >
> > > I think I copied it from the vdagent.c where it was added
> > > in 0ffca2b ("vdagent: Use glib's commandline parser").
> > > I will change it to syslog().
>
> One thing I did not realize while I was responding to you
> review yesterday: If the vdagentd was started with wrong args,
> we probably want to output the "Invalid arguments" message to
> the stderr. However, syslog() does not always do that, it
> depends on the do_daemonize var when we call openlog().
Before this commit, usage() was doing fprintf() on stderr;
g_printerr() will do fputs() on stderr, so it should be fine.
> do_daemonize is set during the parsing. If the parsing fails,
> do_daemonize might not be set correctly. So using g_printerr()
> is correct in this case, IMHO.
Thanks,
>
> Jakub
> >
> > Thanks,
> > Victor
> > > >
> > > > I hope we can move to that whenever we bump glib to 2.50 or above
> > > > as it writes to systemd's journal by default for daemons (AFAIK).
> > > >
> > > > Reviewed-by: Victor Toso <victortoso@xxxxxxxxxx>
> > > >
> > > > Feel free to submit a new version with fixes as standalone patch
> > > > with me in cc, to reduce the backlog of this series ;)
> > > >
> > > > Cheers,
> > > > Victor
> > > >
> > > > > + g_error_free(err);
> > > > > + return 1;
> > > > > }
> > > > >
> > > > > + if (portdev == NULL)
> > > > > + portdev = g_strdup(DEFAULT_VIRTIO_PORT_PATH);
> > > > > + if (vdagentd_socket == NULL)
> > > > > + vdagentd_socket = g_strdup(VDAGENTD_SOCKET);
> > > > > + if (uinput_device == NULL)
> > > > > + uinput_device = g_strdup(DEFAULT_UINPUT_DEVICE);
> > > > > +
> > > > > memset(&act, 0, sizeof(act));
> > > > > act.sa_flags = SA_RESTART;
> > > > > act.sa_handler = quit_handler;
> > > > > @@ -1223,5 +1234,9 @@ int main(int argc, char *argv[])
> > > > > if (do_daemonize)
> > > > > unlink(pidfilename);
> > > > >
> > > > > + g_free(portdev);
> > > > > + g_free(vdagentd_socket);
> > > > > + g_free(uinput_device);
> > > > > +
> > > > > return retval;
> > > > > }
> > > > > --
> > > > > 2.17.1
> > > > >
> > > > > _______________________________________________
> > > > > Spice-devel mailing list
> > > > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > >
> > > Cheers,
> > > Jakub
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel