Merge lp:~hikiko/unity-control-center/unity-control-center.race-cond-empty-monitor-name into lp:unity-control-center

[Eleni Maria Stea]

fixed race condition when the monitor is not detected correctly (and the monitor name is null)
added a mark in 1.0 (default value)

fixed race condition when the monitor is not detected correctly (and the monitor name is null)
added a mark in 1.0 (default value)

[Sebastien Bacher (seb128)]

Thanks for the work! Some review comments

* > g_warning ("Invalid dictionary entry.\n");

the "\n" is not needed with g_warning

While you are doing some code cleaning, desrt pointed those on IRC:

* add_dict_entry(),

" g_variant_get_child (tmp, 0, "s", &str);
    if (!str)
    {
      fprintf (stderr, "Invalid dictionary entry\n");"

that case can be dropped, it's not possible for str to be null

- the malloc() uses should be replaced by g_new(), the function leaks the result

Could you fix at least the warnings? The other comments can be addressed later in another merge then if you prefer to handle cleanups separatly

[Sebastien Bacher (seb128)]

(we agreed to land what is in this mp, once the g_warning are changed, and do the other changes in a followup merge request)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:12731
http://jenkins.qa.ubuntu.com/job/unity-control-center-ci/47/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-control-center-trusty-amd64-ci/47
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-control-center-trusty-armhf-ci/47
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-control-center-trusty-i386-ci/48

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-control-center-ci/47/rebuild

review: Approve (continuous-integration)

[Eleni Maria Stea (hikiko)]

thanks a lot for the review, I ll propose a new branch with the fixes above in a while!

=== modified file 'panels/display/cc-display-panel.c'
--- panels/display/cc-display-panel.c	2014-02-20 21:17:51 +0000
+++ panels/display/cc-display-panel.c	2014-02-24 16:59:15 +0000
@@ -595,7 +595,7 @@
     g_variant_get_child (tmp, 0, "s", &str);
     if (!str)
     {
-      fprintf (stderr, "Invalid dictionary entry\n");
+      g_warning ("Invalid dictionary entry.\n");
       break;
     }
 
@@ -619,14 +619,18 @@
 
   GVariant *dict;
 
+  GtkAdjustment *adj = gtk_range_get_adjustment (GTK_RANGE(self->priv->ui_scale));
   const char *monitor_name = gnome_rr_output_info_get_name (self->priv->current_output);
-
-  GtkAdjustment *adj = gtk_range_get_adjustment (GTK_RANGE(self->priv->ui_scale));
+  if (!monitor_name)
+  {
+    g_warning("Failed to get monitor name.\n");
+    return;
+  }
 
   gtk_adjustment_set_upper (adj, UI_SCALE_MAX);
   gtk_adjustment_set_lower (adj, UI_SCALE_MIN);
-
   gtk_scale_set_digits (GTK_SCALE(self->priv->ui_scale), 0);
+  gtk_scale_add_mark (GTK_SCALE(self->priv->ui_scale), 8, GTK_POS_TOP, NULL);
 
   g_settings_get (self->priv->desktop_settings, "scale-factor", "@a{si}", &dict);
   if (!g_variant_lookup (dict, monitor_name, "i", &value))
@@ -1048,6 +1052,12 @@
     GVariant *dict;
 
     monitor_name = gnome_rr_output_info_get_name (self->priv->current_output);
+    if (!monitor_name)
+    {
+      g_warning("Failed to get monitor name.\n");
+      return FALSE;
+    }
+
     g_settings_get (self->priv->desktop_settings, "scale-factor", "@a{si}", &dict);
     dict = add_dict_entry (dict, monitor_name, value);
     g_settings_set (self->priv->desktop_settings, "scale-factor", "@a{si}", dict);