GNOME Bugzilla – Bug 590991
gpm_prefs_setup_action_combo does not match enumeration of GpmActionPolicy
Last modified: 2009-08-11 16:40:31 UTC
Please describe the problem: Gconf settings are off by one checkbox when setting preferences in gnome-power-manager preferences. For "On Battery Power", "When laptop lid is closed" the options are: Blank Screen Suspend Hibernate Shutdown If I choose Hibernate, --verbose shows - Changing /apps/gnome-power-manager/buttons/lid_battery to suspend choosing shutdown shows hibernate, suspend shows blank, blank shows "nothing". So the option choices are just off-by one and my choice seemed to be ignored but was really stored as the option above it. Soo, choosing Hibernate sets it to suspend. This choice is reflected the next time I open gnome-power-preferences, so it is actually stored. The option box is just off-by-one. Every time I open the application, it loads the options, showing what I chooce -1, saving it on close as shown -1 again, so opening closing, opening will -1 the option also. For some reason hibernate seems to be a default becuase opening and closing a few times seems to have it end up with hibernate. If I set it to hibernate, verifying with --verbose that it actually sets it to suspend, everything works normaly. So the option is stored and acted upon correctly. This bug is merely an interface problem. Steps to reproduce: 1. running GPM preferences in --verbose, chose "hibernate" for "when laptop lid is closed" 2. output will show Changing /apps/gnome-power-manager/buttons/lid_battery to suspend 3. your gconf setting will be set to suspend Actual results: -output will show Changing /apps/gnome-power-manager/buttons/lid_battery to suspend -your gconf setting will be set to suspend Expected results: -output will show Changing /apps/gnome-power-manager/buttons/lid_battery to hibernate -your gconf setting will be set to hibernate Does this happen every time? yes Other information: The problem has been debugged here: https://bugs.launchpad.net/ubuntu/karmic/+source/gnome-power-manager/+bug/407491 I came up with how to make the patch, but I won't have the time to write/test it for a few weeks. Notes to developers (or me if I can get around to it...): in src/gpm-prefs-core.c function "gpm_prefs_setup_action_combo" has the order: shutdown suspend hibernate blank ask nothing but the enumeration is: typedef enum { GPM_ACTION_POLICY_BLANK, GPM_ACTION_POLICY_SUSPEND, GPM_ACTION_POLICY_SHUTDOWN, GPM_ACTION_POLICY_HIBERNATE, GPM_ACTION_POLICY_INTERACTIVE, GPM_ACTION_POLICY_NOTHING } GpmActionPolicy; this causes a problem in: gpm_prefs_action_combo_changed_cb (GtkWidget *widget, GpmPrefs *prefs) because: actions = (const GpmActionPolicy *) g_object_get_data (G_OBJECT (widget), "actions"); active = gtk_combo_box_get_active (GTK_COMBO_BOX (widget)); policy = actions[active]; policy calls the "active" value of the "actions" array, which does not correspond to the checkbox order! Fix: re-enumerate the actions order or re-write the "append" to checkbox list to be the correct order bug possibly originated here: http://git.gnome.org/cgit/gnome-power-manager/commit/?id=8bf6879f00e17d7feb39cecb820ad0ad33e2e827
(Critical because someone's laptop could shutdown when they thought it was set to hibernate, causing data loss)
I tried switching the order of the order in gpm_prefs_setup_action_combo but the problem remained. Selecting an option from the combo box in preferences produces a different result in the --verbose output. The bug must be somewhere else.
Some more debugging: the combo box displays the correct options (lines 356-395 of gpm-prefs-core.c), but the egg-debug call on line 404 shows: TI:00:15:38 TH:0xff0800 FI:gpm-prefs-core.c FN:gpm_prefs_setup_action_combo,404 - added: blank TI:00:15:38 TH:0xff0800 FI:gpm-prefs-core.c FN:gpm_prefs_setup_action_combo,404 - added: suspend TI:00:15:38 TH:0xff0800 FI:gpm-prefs-core.c FN:gpm_prefs_setup_action_combo,404 - added: blank TI:00:15:38 TH:0xff0800 FI:gpm-prefs-core.c FN:gpm_prefs_setup_action_combo,404 - added: suspend TI:00:15:38 TH:0xff0800 FI:gpm-prefs-core.c FN:gpm_prefs_setup_action_combo,404 - added: blank in --verbose. somewhere between lines 395 and 405 we get the 'actions_added' array screwed up. The lines are: actions_added = (GpmActionPolicy *) g_ptr_array_free (array, FALSE); actions_added[n_added] = -1; g_object_set_data_full (G_OBJECT (widget), "actions", (gpointer) actions_added, (GDestroyNotify) gpm_prefs_actions_destroy_cb);
Are you sure you're using upstream svn master? This commit should have fixed things before 2.27.5 was released: commit 0a257a46668d91a048b93cc89a7b45b9fdb6d564 Author: Richard Hughes <richard@hughsie.com> Date: Sat Aug 1 18:11:05 2009 +0100 Fix up the setting of the combo-boxes in gnome-power-preferences
Thanks for checking on this. This bug appeared before the git master on 2009/07/29 and continued through 2.27.5 up to today's git master. There are multiple reporters on launchpad describing this bug in 2.27.5. I have a laptop with this bug with 2.27.5 and a desktop with the build from git master that has this bug. I asked a reporter on LP to test with git master, and they also reported that it still exists as today's master (Aug 7). This bug exists when changing any of the action boxes in the preferences UI, and can be seen by comparing the --verbose output with the combo box selection. This is also seen when we populate the combo boxes. Although the UI shows the correct options, the egg_debug shows the list of what each box was supposedly populated with, which does not match what is in the combo box.
I really cannot reproduce this bug -- even disabling suspend and hibernate options manually, things seem to still work. Maybe if you can reproduce this you can debug this and find the issue? Thanks.
I can reproduce this with current 2.27.5. Actually, contrary to other reports, when I run with --verbose I see that any setting I choose in the UI for "When lid is closed" other then "hibernate" causes gpm_prefs to choose "blank". when choosing "hibernate" it sets "suspend". Its also consistent when started - if gconf has lid_battery or lid_ac set to "suspend", then when gnome-power-preferences is started it shows "hibernate" as the relevant option; If "blank" is set then the UI shows "shutdown" (which is interestingly not a suggested option according to gconf's "Long description"); if "hibernate" is set then the the UI shows the selector to be blank (weird) and if "nothing" is set then the UI shows "Do nothing" which is otherwise not an available option. While I haven't tested all the options using gconf (only blank and suspend) I assume it behaves properly according to the gconf settings and its just the UI that is out of sync.
All reporters have 64 bit architectures, and every computer I tested had 64 bit architectures - and they all had this bug. I purged Ubuntu's GPM and have only been using my own compiled git master. I added egg_debug calls to follow the data and found something funky going on with the pointers (specifically on line 397 actions_added does not resemble the array of policies in "array"). Unfortunately, my day job prevents me from working on it as much as I'd like, so I'll present what I have and possibly work on a patch later today - but if the fix is obvious and easy to someone, please go ahead and do it. This is possibly a typsetting problem on 64 bit architectures in gpm-prefs-core.c: lines 356-390 use: g_ptr_array_add (array, GINT_TO_POINTER (policy)); which adds the value of "policy" (an int) into an array of pointers to longs (GINT_TO_POINTER actually does ((gpointer) (long) (int)). "array" is now an array of pointers to longs. The long value of each pointer is 32 bits (instead of the full 64 bits) of the value of the int "policy". line 397: actions_added = (GpmActionPolicy *) g_ptr_array_free (array, FALSE); "g_ptr_array_free" returns an array of pointers to longs, which are typecasted into an array of pointers to "GpmActionPolicy"s, which are ints. ***Therefore, we are typecasting an array of pointers to longs to an array of pointers to ints.*** This could be the problem. This will only be seen in 64 bit systems since 32 bit systems have 32 bit ints and longs while 64 bit systems have 32 bit longs and 64 bit ints. line 399: g_object_set_data_full (G_OBJECT (widget), "actions", (gpointer) actions_added, (GDestroyNotify) gpm_prefs_actions_destroy_cb); Here is where we pass actions_added to set the "actions" for each of the combo box choices. actions added could be an array of pointers to longs typecasted to an array of pointers to ints.
(In reply to comment #8) > All reporters have 64 bit architecture Ahh, I didn't test that. > actions_added = (GpmActionPolicy *) g_ptr_array_free (array, FALSE); > > "g_ptr_array_free" returns an array of pointers to longs, which are typecasted > into an array of pointers to "GpmActionPolicy"s, which are ints. ***Therefore, > we are typecasting an array of pointers to longs to an array of pointers to > ints.*** This could be the problem. And was. I've fixed this in: commit 8ae9438e8bfbcf2aca3ae787185b2200eb4f59d2 Author: Richard Hughes <richard@hughsie.com> Date: Tue Aug 11 17:38:25 2009 +0100 Add the enumerated actions in a 64bit safe way. Fixes #590991 Thanks for the debugging.