python_modules/demarshal.py and marshal.py fixes for gcc 4.6.0
warning about set but unused variables. The fixes disable creating
of variables mem_size when they are not used (demarshall) and
declaring a src variable when the message doesn't use it (marshal).
You need to touch *.proto after applying this (should add a Makefile
dependency).
CC sw_canvas.lo
cc1: warnings being treated as errors
../common/sw_canvas.c: In function 'canvas_draw_text':
../common/sw_canvas.c:1037:16: error: 'pos.x' may be used uninitialized in this function
../common/sw_canvas.c:1037:16: error: 'pos.y' may be used uninitialized in this function
make[4]: *** [sw_canvas.lo] Error 1
We don't know yet what will be the guest previous configuration.
Ie, what should we send otherwise? Current hardware configuration?
This works badly with windowed mode, where we expect the same windows
to be displayed on reconnection.
Since it's unclear yet how MonitorConfig should be used depending on
use case (full-screen vs windowed) we prefer to make a public api so
that the client implementation can send it when it is the most
appropriate time.
clipboard_by_guest tracking was used more or less for 2 things, to keep track
if the agent has clipboard data ready to send, and to see if we have done a
clipboard_set_with_data on behalf of the guest agent.
This patch splits the tracking of the 2, fixing several issues:
1) spice_display_paste_from_guest would not work if since receiving
the grab from the agent some other app has copied something to
the client clipboard.
2) We would do a clipboard_clear unconditionally even if we were
not the clipboard owner in the client (iow some other app has
done a clipboard_set_with_data since out last one).
This patch changes the meaning of the clipboard_by_guest boolean to just
track if we've done a clipboard_set_with_data on behalf of the guest
and are the last one to have a done a clipboard_set_with_data (iow we are the
client os' clipboard owner). It adds a checks to clipboard_release to
only call clipboard_clear if we are the current ownerm fixing 1).
This patch uses nclip_targets to keep track of the agent having data
available which we could paste, fixing 2).
When we call gtk_clipboard_set_with_data to set the client clipboard
to the targets reported as available by the agent, the clipboard no
longer has data in the sense that it has data which is interesting
for spice_display_copy_to_guest, so clear clip_hasdata whenever we
call gtk_clipboard_set_with_data,
By setting d->clip_grabbed[selection] to FALSE when we receive a grab from
the agent, we can remove the weird "if (!d->clipboard_by_guest[selection])"
check from clipboard_owner_change, and we fix spice_display_copy_to_guest not
working in the following case:
1) autoclipboard share disabled
2) Copy something to the clipboard in the client
3) Send it to the guest by calling spice_display_copy_to_guest
4) Copy something to the clipboard in the guest
5) Tried to send the client clipboard to the guest again by calling
spice_display_copy_to_guest (again).
5) would not work because d->clip_grabbed[selection] would still be true in
spice-gtk's view, where as the agent no longer sees the clipboard as grabbed
by the client since it send a grab itself.
Also change 0/1 to FALSE/TRUE in touched code. spice-widget seems to be
using all 3 of: 0/1 false/true and FALSE/TRUE for booleans. The glib convention
is FALSE/TRUE.
08:55 < hansg> elmarco, this is from vdagent.log with debugging enabled. What is happening is that the last thing done was a primary
selection in the client, so vdagent owns the clipboard in the guest (on behalf of spice-gtk), then something gets selected
inside the guest, the agent sends a grab to spice-gtk, which then does a gtk_clipboard_set_with_data, this triggers an
clipboard_owner_change which sends a release message to the agent
08:56 < hansg> To which the agent then responds by dropping it, and logging:
08:56 < hansg> primary: received release while not owning client clipboard
09:11 < elmarco> hansg: but this bug we are talking about is not related to multi-clipboard right?
09:11 < elmarco> and it's only a warning in vdagent, things works as expected otherwise, right?
09:11 < elmarco> the bug was thee before I suppose
09:12 < hansg> right, they work because vdagent is diligent and sees the client sends a release while it is not owning the clipboard. The
diligence is mainly there in case things race though (release on client racing with a grab on guest), not to make things
work with buggy clients :)
09:13 < hansg> wrt: <elmarco> hansg: d->clip_grabbed is only for client-side grab, iirc
09:13 < elmarco> ok, I think it's just an obscure area of the spec, where basically, we don't define exactly the "state machine"
09:13 < elmarco> so the client is releasing his last client-side grab, because he had one before, but now, it is a guest grab
09:13 < hansg> True (not exactly definging the state machine)
09:14 < hansg> So to try again wrt the d->clip_grabbed, what happens there (which has the same cause) is:
09:14 < elmarco> so, what it should do is just don't release the clipboard if it is switching from client-side to guest
09:15 < hansg> gtk/spice-widget.c: clipboard_grab gets called, and does:
09:15 < hansg> Hmm, hold on, I see what you mean wrt d->clip_grabbed now
09:16 < elmarco> to me, it looks like the client made a grab and to complete it's cycle, it should release his grab
09:16 < elmarco> but the order of things confuse vdagent and we should agree on something and document it
09:18 < hansg> elmarco, I need some time to take a somewhat closer look at the spice-gtk code in this area, give me 1/2 an hour and I'l
get back to you
09:20 < elmarco> from client 1. grab -> 2. grab <- 3. release -> or 1. grab -> 2. grab <- 3. no release
09:21 < elmarco> I think state should not be mixed between client grab / release -> and guest grab/release <-
09:22 < elmarco> so, overriding client grab by guest grab should release client grab
09:23 < hansg> spicec and the linux agent both assume that after sending a grab they won't get back a release (for that selection). The
purpose of the release is to tell the OS that the agent resp, client no longer own the clipboard (by setting the owner to
None under X11), so that other apps can disable their paste menu item, etc. There is no need to do that (and actually
doing so would be a bug) if an other app now owns the clipboard. So if the other side claim
09:23 < hansg> s ownership of the clipboard there is no need to tell it you're releasing your side, since it already assume you have
09:24 < hansg> Scenarios to keep in mind are:
09:24 < hansg> Seen from the client side:
09:28 < hansg> client grabs clipboard
09:28 < hansg> some app on guest becomes owner, guest sends grab, assume client release
09:29 < hansg> some app on guest asks agent for clipboard data -> tells it to go away since the client no longer the owner
09:29 < hansg> If it would not assume the release, there would be a window where it would think the client still owns the clipboard and
forward potential request to the client, even though the client no longer owns the clipboard
09:30 < hansg> The thing to keep in mind is that the delivery of messages is not instant, so there is some window where the 2 sides are
out of sync.
09:30 < hansg> I can see the logic in how you're advocating to do things, but that is not how they are currently done and I'm reluctant
to change this
09:33 < elmarco> hansg: yeah, I don't think one solution or the other affect user experience, for me there is no gap if the client and
agent agrees, it's only protocol/implementation details
09:33 < elmarco> since there was prior implementation, we can decide to follow it
spice-gtk configure.ac has some code to detect if the compiler has
a special attribute to tag some functions so that they generate a
warning when their return value isn't checked. However, this test
is broken (the gcc attribute name is "warn_unused_result", not
"__warn_unused_result__" and WARN_UNUSED_RESULT is unused anyway
since spice-protocol provides SPICE_GNUC_WARN_UNUSED_RESULT. Thus
we can just drop that block of code from configure.ac
configure.ac had -fvisibility detection, but it's not used by
spice-gtk. It also has a --enable-static-linkage flag which isn't
used anywhere apart from in configure.ac, so remote this too. I
think the same effect as --enable-static-linkage can be achieved
using make LDFLAGS="-all-static" since we are using libtool.
