After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 679444 - wacom: gnome-settings-daemon aborts when a screen tablet is plugged in but not enabled
wacom: gnome-settings-daemon aborts when a screen tablet is plugged in but no...
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: wacom
3.5.x
Other Linux
: Normal major
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2012-07-05 13:20 UTC by Olivier Fourdan
Modified: 2012-07-05 16:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (1.50 KB, patch)
2012-07-05 13:22 UTC, Olivier Fourdan
reviewed Details | Review
Updated patch (1.03 KB, patch)
2012-07-05 15:47 UTC, Olivier Fourdan
accepted-commit_now Details | Review

Description Olivier Fourdan 2012-07-05 13:20:14 UTC
gsd_wacom_device_get_display_rotation() (fix for bug 668547, git commit ecb0e2d0) does not check if the crtc returned by gnome_rr_output_get_crtc() is not NULL, which is the case when the output is first connected but not enabled.

Note: This fix will be needed in gnome-control-center as well.
Comment 1 Olivier Fourdan 2012-07-05 13:22:15 UTC
Created attachment 218081 [details] [review]
Proposed patch

Proposed fix
Comment 2 Bastien Nocera 2012-07-05 14:53:28 UTC
Review of attachment 218081 [details] [review]:

::: plugins/wacom/gsd-wacom-device.c
@@ +913,2 @@
 	rr_output = find_output (rr_screen, device);
+	if (rr_output == NULL)

Would it be simpler to do:
if (rr_output) {
   GnomeRRCrtc *crtc = gnome_rr_output_get_crtc (rr_output);
   if (crtc)
      rotation = gnome_rr_crtc_get_current_rotation (crtc);
}
And leave the rest as-is?
Comment 3 Olivier Fourdan 2012-07-05 15:47:25 UTC
Created attachment 218107 [details] [review]
Updated patch

That was my first implementation as well, but realized I am no big fan of nested tests so changed it eventually.

Admittedly, that one's much simpler though.
Comment 4 Bastien Nocera 2012-07-05 16:16:52 UTC
Seeing as it's one level deep, and makes the function not use a label, I consider it a win :)

Looks good good to commit, thanks!
Comment 5 Olivier Fourdan 2012-07-05 16:29:30 UTC
Cheers! Committed in g-s-d and updated in g-c-c as well

commit f682df67fd3689da85f488d6919cb197467ea07c

    wacom: check if crtc is valid when querying rotation
    
    The value returned by gnome_rr_crtc_get_current_rotation()
    can be NULL if the output is not enabled.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=679444