Merge lp:~charlesk/lightdm/lp-1235915 into lp:lightdm

[Charles Kerr]

don't call xkl_config_rec_get_from_server if we don't have a valid xkl_engine.

don't call xkl_config_rec_get_from_server if we don't have a valid xkl_engine.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1931
http://jenkins.qa.ubuntu.com/job/lightdm-ci/255/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/lightdm-trusty-amd64-ci/49
    SUCCESS: http://jenkins.qa.ubuntu.com/job/lightdm-trusty-armhf-ci/49

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/lightdm-ci/255/rebuild

review: Approve (continuous-integration)

[Robert Ancell (robert-ancell)]

I'd prefer it like this:

     xkl_engine = xkl_engine_get_instance (display);
     if (!xkl_engine)
     {
         g_warning ("Failed to get Xkl engine for display '%p'", display);
         return laoyuts;
     }

i.e. no need to indent the entire function if nothing happens if the engine is NULL.

review: Needs Fixing

[Robert Ancell (robert-ancell)]

Is there a particular bug this is fixing? Looking at the xklavier source xkl_engine_get_instance should always return an object except in the case where display is NULL (which should probably be a g_assert).

Perhaps the correct fix here is to check if XOpenDisplay is NULL instead?

[Robert Ancell (robert-ancell)]

Oh, duh, now I see the bug.

[Robert Ancell (robert-ancell)]

And I see you came to much the same conclusion... So yeah, just make it check if XOpenDisplay has failed and then return from the function.

[lp:~charlesk/lightdm/lp-1235915]

1932. By Charles Kerr on 2014-03-17

simplify the patch based on Robert's feedback

[Charles Kerr (charlesk)]

Simplified r1932.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1932
http://jenkins.qa.ubuntu.com/job/lightdm-ci/258/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/lightdm-trusty-amd64-ci/52
    SUCCESS: http://jenkins.qa.ubuntu.com/job/lightdm-trusty-armhf-ci/52

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/lightdm-ci/258/rebuild

review: Approve (continuous-integration)

[Robert Ancell (robert-ancell)]

review: Approve

=== modified file 'liblightdm-gobject/layout.c'
--- liblightdm-gobject/layout.c	2013-01-31 20:56:09 +0000
+++ liblightdm-gobject/layout.c	2014-03-17 17:11:38 +0000
@@ -115,6 +115,9 @@
         return layouts;
 
     display = XOpenDisplay (NULL);
+    if (display == NULL)
+        return NULL;
+    
     xkl_engine = xkl_engine_get_instance (display);
     xkl_config = xkl_config_rec_new ();
     if (!xkl_config_rec_get_from_server (xkl_config, xkl_engine))