Created attachment 355461 [details] [review]
clutter/x11: Set master device on pad button events

These should be set one, but just set the master to be the slave
pad device. We are passively grabbing the pad device, so this is
consistent with active grabs on slave devices. Besides, pads are
paired to the VCP, which is not really truthful.

Fixes inoffensive warnings when trying to check whether motion
throttling applies for these events.

Created attachment 355462 [details] [review]
backends: Fix dangling callback not being disconnected

This fixes possible crashes if configuration is changed on a device that
was added and then removed.

Created attachment 355463 [details] [review]
backends: Set error when opening /sys file fails

The caller in clutter really expects an error if fd==-1, so make
sure we set one here. Otherwise we get a nice crash in addition to
the failure to open the /sys file.

Review of attachment 355463 [details] [review]:

::: src/backends/native/meta-launcher.c
@@ +199,3 @@
+                       G_IO_ERROR,
+                       G_IO_ERROR_NOT_FOUND,
+                       "Could not open /sys file: %s: %m", path);

This could use G_FILE_ERROR and g_file_error_from_errno (errno), and return -1 for clarity. Otherwise looks good.

Review of attachment 355462 [details] [review]:

Makes sense.

Review of attachment 355461 [details] [review]:

::: clutter/clutter/x11/clutter-device-manager-xi2.c
@@ +466,3 @@
   if (source == CLUTTER_PAD_DEVICE)
+    {
+      get_pad_features (info, &num_rings, &num_strips);

Can't seem to find this function anywhere. Does these patches depend on something else?

(In reply to Jonas Ådahl from comment #4)
> Review of attachment 355463 [details] [review] [review]:
> 
> ::: src/backends/native/meta-launcher.c
> @@ +199,3 @@
> +                       G_IO_ERROR,
> +                       G_IO_ERROR_NOT_FOUND,
> +                       "Could not open /sys file: %s: %m", path);
> 
> This could use G_FILE_ERROR and g_file_error_from_errno (errno), and return
> -1 for clarity. Otherwise looks good.

Right about G_FILE_ERROR. I briefly thought about using errno here, but other warnings and error paths settle with %m, probably not a big leap since errno.h is already included.

(In reply to Jonas Ådahl from comment #6)
> Review of attachment 355461 [details] [review] [review]:
> 
> ::: clutter/clutter/x11/clutter-device-manager-xi2.c
> @@ +466,3 @@
>    if (source == CLUTTER_PAD_DEVICE)
> +    {
> +      get_pad_features (info, &num_rings, &num_strips);
> 
> Can't seem to find this function anywhere. Does these patches depend on
> something else?

Oops, that belongs to other patchset (bug #782033), I'll fix it up when cherry-picking/pushing on master, since this will make it first.

Attachment 355461 [details] pushed as 56c468a - clutter/x11: Set master device on pad button events
Attachment 355462 [details] pushed as 4b8dd51 - backends: Fix dangling callback not being disconnected

Created attachment 355540 [details] [review]
backends: Set error when opening /sys file fails

The caller in clutter really expects an error if fd==-1, so make
sure we set one here. Otherwise we get a nice crash in addition to
the failure to open the /sys file. Also, retry on EINTR.

Review of attachment 355540 [details] [review]:

::: src/backends/native/meta-launcher.c
@@ +201,3 @@
+                       "Could not open /sys file: %s: %m", path);
+          return -1;
+        }

I usually see this being implemented as a do-while loop, rather than goto/labels:

do
  {
    fd = open (path, flags);
  }
while (fd < 0 && errno == EINTR);

if (fd < 0)
  {
    g_set_error (...);
    return -1;
  }

..

Yup, that looks less alien :). Changed and pushed.

Attachment 355540 [details] pushed as a6ec2b1 - backends: Set error when opening /sys file fails