GNOME Bugzilla – Bug 132655
[PATCH] cleanup error msgs wrt /dev/pmu on powerbooks
Last modified: 2006-01-11 18:38:38 UTC
Attached patch cleans up the error reporting wrt /dev/pmu (whether it exists, has the right permissions etc).
Created attachment 23798 [details] [review] acme-fblevel patch
The patch looks good to me, but I haven't tested it.
Created attachment 23800 [details] [review] acme-fblevel patch
New patch doesn't display a dialog when the machine is not a powerbook. The error is ignored.
Notes: Changed priority to high for increased visibility. Added bugsquad keyword.
String freeze breakage, we'll apply that to 2.7 I'm afraid.
I'm adding the BLOCKED_BY_FREEZE keyword.
Hi It seems that a small (typo ?) error in this patch. The test on error in acme_fblevel_new is wrong. To work : g_return_val_if_fail (error == NULL, NULL); should become g_return_val_if_fail (*error == NULL, NULL);
We shouldn't even have that test in there, it'll get removed.
according to the pmud debian maintainer using directly /dev/pmu is dangerous because concurrent access to /dev/pmu is not supported. pmud/pbbuttonsd is supposed to mediate all access to /dev/pmu ... so it would be nice to use that rather
control-center has branched, so this is no longer blocked by freeze and I'm removing the keyword. The patch should probably be either applied, or changed to needs-work based on Sebastien's comments.
Seb, should this be marked needswork instead of commit after freeze then?
Comment on attachment 23800 [details] [review] acme-fblevel patch yep, probably. I've updated the status.
Cleaning up milestones. Would be good to get this in, but not a global gnome showstopper.
The Debian/Ubuntu package still use that patch. Anything should be changed to get it considered for upstream? It's marked as "needs-work", what change should be done?
After removing this particular line: + g_return_val_if_fail (error == NULL, NULL); which is unneeded, the patch looks fine.
Created attachment 57169 [details] [review] patch updated according to previous comment 2006-01-11 Sebastien Bacher <seb128@debian.org> reviewed by: Bastien Nocera <hadess@hadess.net> * actions/acme-fb-level.c: (acme_fblevel_error_quark), (acme_fblevel_new): * actions/acme-fb-level.h: * gnome-settings-multimedia-keys.c: (gnome_settings_multimedia_keys_load): cleanup messages about pmu on powerbook, patch by Jeroen Zwartepoorte <jeroen.zwartepoorte@gmail.com> (Closes: #132655)
thanks for your work on that