[Xfce-bugs] [Bug 5377] New: libxfce4gui session-client bug fixes, improvements, and some optimizations

[bugzilla-daemon at xfce.org]

http://bugzilla.xfce.org/show_bug.cgi?id=5377

           Summary: libxfce4gui session-client bug fixes, improvements,
                    and some optimizations
           Product: Xfce
           Version: 4.6.1
          Platform: PC (x86)
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: Medium
         Component: general
        AssignedTo: xfce-bugs at xfce.org
        ReportedBy: hamster at mbox.contact.bg


Created an attachment (id=2361)
 --> (http://bugzilla.xfce.org/attachment.cgi?id=2361)
All the changes listed above

I wanted to add X11 session support to mousepad, but ran into some gui4
session-client bugs and deficiencies, and had to hack it a bit.


Bug fixes:

client_session_new_full() sets the restart, clone etc. commands _by directly
assigning to them_ instead of duplicating the arguments passed. So any call
to set_whatever_command() or client_session_free() crashes when attempting
to g_strfreev() the command(s), unless the **values passed to new_full() are
allocated-and-not-freed by the caller. However, client_session_new() passes
the same array for both restart and clone, and the array elements are taken
directly from argv[], with obvious results.

When client_session_new() generates the initial restart command, it does not
strip the (previous) client id, if any, so set_clone_restart_commands() does
not append the (newly given) client id. Of course, a when previous id
exists, the new id will normally be the same - unless, for example, previous
is invalid.

If client_session_free() is called before session_init(), or after an
unsuccessful session_init(), it calls disconnect(), which calls
SmcCloseConnection() with session_connection = NULL and thus crashes.


Improvements (well, hopefully):

There was no way to cancel the shutdown from interact. For compatibility,
I extended the session client structure with gboolean interact_2.

The restart, clone etc. commands are set before invoking the application's
save_yourself/save_phase_2 callbacks. SM spec says that interact and
save_phase_2 should SetProperties, and the session client code does than,
but without giving the application a change to set/alter them. And without
that, for example, mousepad will have to set_restart_command() on each
possible filename change (4 calls in callbacks.c and one more in main.c).
So I moved the set_clone_restart_commands() invocation immediately before
SmcSaveYourselfDone().

The --display (if missing) was appended only to the automatically generated
client_session_new() restart/clone commands, and not to any commands set
by the application. So I moved appending to set_clone_restart_commands().

client_session_new() initializes both clone and restart commands, so if the
application sets a new restart command, it can not rely on the automatic
restart -> clone copy. Altered client_session_new() to pass NULL, so the
latest restart command is copied to clone.

Added the missing client_session_set_interact_style() function. Currently,
all SessionClient fields are accessed via functions, except for
interact_style which must be directly assigned to.


Optimizations:

The largely identical code for setting the different commands in
set_clone_restart_commands() is rebased a single function:

set_property_from_command(client, command [x11 command name], ptr [command
value], add_sm_id, add_display)

The identical set_whatever_command()-s are now one-liners invoking the new
copy_command() function.


Notes:

Using g_strdupv() instead of safe_strvdup() seems to work just fine. Not
knowing why safe_strvdup() stands for, I left it as is.

IMHO, SessionClient should only include a single session id variable, namely
client_id, which should be equal to the previous id before init and to the
new [given] id after; given_client_id should be a local variable of
session_init(). But I didn't want to break compatibility.

I tried to keep the code simple, since the session management is quite
unpleasant by itself (to avoid a harsher expression).

Last but not least, I compiled libxfcegui4.so and replaced my original
shared library. No crashes, and the new library removed --sm-client-id from
the clone commands of xfdesktop and xfwm.


Well, that's about it. If you have any questions, or there are things you
don't like, I'm open for discussion.

-- 
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.