[Spice-devel] [vdagent-linux v2 4/6] console-kit: do not use empty session

[Victor Toso]

Hi,

On Wed, Apr 27, 2016 at 02:49:22PM -0500, Jonathon Jongsma wrote:
> On Sat, 2016-04-23 at 12:27 +0200, Victor Toso wrote:
> > DBus might return empty string for active-session, probably in a non user
> > session (e.g. gdm). It is better to keep the variable as NULL as we check
> > against that (but never against empty strings).
> > ---
> >  src/console-kit.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/console-kit.c b/src/console-kit.c
> > index fc73cf2..7096609 100644
> > --- a/src/console-kit.c
> > +++ b/src/console-kit.c
> > @@ -358,7 +358,12 @@ static char
> > *console_kit_check_active_session_change(struct session_info *si)
> >          }
> >  
> >          dbus_message_iter_get_basic(&iter, &session);
> > -        si->active_session = strdup(session);
> > +        if (session != NULL && session[0] != '\0') {
> > +            si->active_session = strdup(session);
> > +        } else if (si->verbose) {
> > +            syslog(LOG_WARNING, "(console-kit) received invalid session. "
> > +                   "No active-session at the moment");
> > +        }
> >          dbus_message_unref(message);
> >  
> >          /* non blocking read of the next available message */
> 
> Looks OK, but I find it a bit odd to only print a WARNING message if verbose is
> set. usually 'verbose' settings only apply to DEBUG- or INFO-level messages, not
> warnings. If it's only printed in verbose mode, it doesn't seem like it's truly
> a warning. So I'd suggest either changing the severity to LOG_INFO or removing
> the verbose check.

Agreed. I'll change it, thanks!

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