> On Mon, Dec 10, 2018 at 07:42:35AM -0500, Frediano Ziglio wrote: > > > > > > From: Victor Toso <me@xxxxxxxxxxxxxx> > > > > > > Spice client should listen to SpiceMainChannel::agent-connected > > > notification and avoid calling any clipboard related functions such as > > > spice_gtk_session_paste_from_guest() from client-gtk library. > > > > > > This patch removes the silent return of agent_clipboard_grab() in > > > order to properly catch bugs with critical messages. > > > > > > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx> > > > > Documentation for agent-connected: "Whether the agent is connected" > > > > Documentation for main-agent-update signal: > > > > /** > > * SpiceMainChannel::main-agent-update: > > * @main: the #SpiceMainChannel that emitted the signal > > * > > * Notify when the %SpiceMainChannel:agent-connected or > > * %SpiceMainChannel:agent-caps-0 property change. > > **/ > > > > Documentation for spice_main_channel_clipboard_selection_grab > > > > /** > > * spice_main_channel_clipboard_selection_grab: > > * @channel: a #SpiceMainChannel > > * @selection: one of the clipboard #VD_AGENT_CLIPBOARD_SELECTION_* > > * @types: an array of #VD_AGENT_CLIPBOARD types available in the clipboard > > * @ntypes: the number of @types > > * > > * Grab the guest clipboard, with #VD_AGENT_CLIPBOARD @types. > > * > > * Since: 0.35 > > **/ > > > > Documentation for spice_gtk_session_paste_from_guest > > > > /** > > * spice_gtk_session_paste_from_guest: > > * @self: #SpiceGtkSession > > * > > * Copy guest clipboard to client-side clipboard. > > * > > * Since 0.8 > > **/ > > > > I don't see any requirement for the client to check agent status. > > You mean that we should improve documentation or something else? > I was expecting a "is documented here". As spice-gtk is a public library with an API speaking of "Spice client" is not a good thing, should be Spice clientS, meaning all possible clients and being ALL would mean even unknown/proprietary ones. As all clients should respect API documentation you are allow to change API behaviour where the behaviour was documented as not defined (for instance free() behaviour was changed in respect to NULL from undefined behaviour to ignored). At least we should check that the "expected" clients (remote-viewer, virt-viewer, virt-manager I would say) already have this behaviour before changing API behaviour. Is only a warning change but does not help if the change is producing hundred of warnings which would hide any possible issue raised by warnings, not having the agent running is not an impossible use case. > As always, thanks for reviewing. > > > > > > --- > > > src/channel-main.c | 4 +--- > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > diff --git a/src/channel-main.c b/src/channel-main.c > > > index 223043a..aa563d2 100644 > > > --- a/src/channel-main.c > > > +++ b/src/channel-main.c > > > @@ -1352,9 +1352,7 @@ static bool agent_clipboard_grab(SpiceMainChannel > > > *channel, guint selection, > > > size_t size; > > > int i; > > > > > > - if (!c->agent_connected) > > > - return false; > > > - > > > + g_return_val_if_fail(c->agent_connected, false); > > > g_return_val_if_fail(test_agent_cap(channel, > > > VD_AGENT_CAP_CLIPBOARD_BY_DEMAND), false); > > > > > > size = sizeof(VDAgentClipboardGrab) + sizeof(uint32_t) * ntypes; > > > > Frediano > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel