Hi,
On Thu, Jan 10, 2019 at 01:55:37PM -0600, Jonathon Jongsma wrote:
> This patch looks reasonable to me, but I wonder if you could expand the
> commit log to indicate the steps required to trigger the warning? In
> what situation is this function (clipboard_get()) being called when the
> agent is not connected?
Sorry if I did not put it well in the commit log, I did not hit
it in git master (or I not recall doing so). I meant that
spice_main_channel_clipboard_selection_request() is being called
before checking if agent is connected which would trigger the
critical message.
Should be safe to remove the check of agent_connected in
clipboard_get()
> Jonathon
>
> On Thu, 2019-01-10 at 16:39 +0100, Victor Toso wrote:
> > From: Victor Toso <me@xxxxxxxxxxxxxx>
> >
> > In case the agent is disconnected, we we don't need to create the
> > struct RunInfo, GMainLoop and add handlers to some signals.
> >
> > Before this patch, spice_main_channel_clipboard_selection_request()
> > was always called even if agent was not connected, which would log
> > a critical message in agent_clipboard_request() and unnecessary call
> > to spice_channel_wakeup().
> >
> > This patch also removes one goto and related cleanup label.
> >
> > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> > ---
> > src/spice-gtk-session.c | 14 ++++++--------
> > 1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > index 1ccae07..373d11e 100644
> > --- a/src/spice-gtk-session.c
> > +++ b/src/spice-gtk-session.c
> > @@ -728,6 +728,12 @@ static void clipboard_get(GtkClipboard
> > *clipboard,
> > g_return_if_fail(info < SPICE_N_ELEMENTS(atom2agent));
> > g_return_if_fail(s->main != NULL);
> >
> > + g_object_get(s->main, "agent-connected", &agent_connected,
> > NULL);
> > + if (!agent_connected) {
> > + SPICE_DEBUG("Request to guest failed as agent is not
> > running");
> > + return;
> > + }
> > +
> > ri.selection_data = selection_data;
> > ri.info = info;
> > ri.loop = g_main_loop_new(NULL, FALSE);
> > @@ -744,13 +750,6 @@ static void clipboard_get(GtkClipboard
> > *clipboard,
> > spice_main_channel_clipboard_selection_request(s->main,
> > selection,
> > atom2agent[info].
> > vdagent);
> >
> > -
> > - g_object_get(s->main, "agent-connected", &agent_connected,
> > NULL);
> > - if (!agent_connected) {
> > - SPICE_DEBUG("canceled clipboard_get, before running loop");
> > - goto cleanup;
> > - }
> > -
> > /* This is modeled on the implementation of gtk_dialog_run()
> > even though
> > * these thread functions are deprecated and appears to be
> > needed to avoid
> > * dead-lock from gtk_dialog_run().
> > @@ -761,7 +760,6 @@ static void clipboard_get(GtkClipboard
> > *clipboard,
> > gdk_threads_enter();
> > G_GNUC_END_IGNORE_DEPRECATIONS
> >
> > -cleanup:
> > g_clear_pointer(&ri.loop, g_main_loop_unref);
> > g_signal_handler_disconnect(s->main, clipboard_handler);
> > g_signal_handler_disconnect(s->main, agent_handler);
>
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel