vfs301: Fix device pointer handling in callback

When porting the driver to the new libfprint 1.90.0 a mistake was made
where the device was not passed through user_data anymore but it was
still read from there. Stop using user_data in the callback to fix this.

Closes: #320

libfprint/drivers/aes1610.c

@@ -135,10 +135,8 @@ generic_read_ignore_data (FpiSsm *ssm, FpDevice *dev,
                           size_t bytes)
 {
   FpiUsbTransfer *transfer = fpi_usb_transfer_new (dev);
-  unsigned char *data;
 
-  data = g_malloc (bytes);
-  fpi_usb_transfer_fill_bulk_full (transfer, EP_IN, data, bytes, g_free);
+  fpi_usb_transfer_fill_bulk (transfer, EP_IN, bytes);
   transfer->ssm = ssm;
   transfer->short_is_error = TRUE;
   fpi_usb_transfer_submit (transfer, BULK_TIMEOUT, NULL,

libfprint/drivers/aes3500.c

@@ -106,7 +106,9 @@ static struct aes_regwrite init_reqs[] = {
   { 0x9e, 0x53 },       /* clear challenge word bits */
   { 0x9f, 0x6b },       /* set some challenge word bits */
   { 0, 0 },
+};
 
+static struct aes_regwrite capture_reqs[] = {
   { 0x80, 0x00 },
   { 0x81, 0x00 },
   { 0, 0 },
@@ -155,4 +157,6 @@ fpi_device_aes3500_class_init (FpiDeviceAes3500Class *klass)
   aes_class->enlarge_factor = ENLARGE_FACTOR;
   aes_class->init_reqs = init_reqs;
   aes_class->init_reqs_len = G_N_ELEMENTS (init_reqs);
+  aes_class->capture_reqs = capture_reqs;
+  aes_class->capture_reqs_len = G_N_ELEMENTS (capture_reqs);
 }

libfprint/drivers/aes3k.c

@@ -42,7 +42,10 @@
 
 typedef struct
 {
-  GCancellable *img_trf_cancel;
+  /* This is used both as a flag that we are in a capture operation
+   * and for cancellation.
+   */
+  GCancellable *img_capture_cancel;
 } FpiDeviceAes3kPrivate;
 
 #define CTRL_TIMEOUT 1000
@@ -83,6 +86,9 @@ img_cb (FpiUsbTransfer *transfer, FpDevice *device,
   FpImage *img;
   int i;
 
+  /* Image capture operation is finished (error/completed) */
+  g_clear_object (&priv->img_capture_cancel);
+
   if (error)
     {
       if (g_error_matches (error,
@@ -91,13 +97,11 @@ img_cb (FpiUsbTransfer *transfer, FpDevice *device,
         {
           /* Cancellation implies we are deactivating. */
           g_error_free (error);
-          g_clear_object (&priv->img_trf_cancel);
           fpi_image_device_deactivate_complete (dev, NULL);
           return;
         }
 
       fpi_image_device_session_error (dev, error);
-      g_clear_object (&priv->img_trf_cancel);
       return;
     }
 
@@ -122,11 +126,10 @@ img_cb (FpiUsbTransfer *transfer, FpDevice *device,
   fpi_image_device_image_captured (dev, img);
 
   /* FIXME: rather than assuming finger has gone, we should poll regs until
-   * it really has, then restart the capture */
+   * it really has. */
   fpi_image_device_report_finger_status (dev, FALSE);
 
-  /* Note: We always restart the transfer, it may already be cancelled though. */
-  do_capture (dev);
+  /* Note: The transfer is re-started when we switch to the AWAIT_FINGER_ON state. */
 }
 
 static void
@@ -141,31 +144,51 @@ do_capture (FpImageDevice *dev)
   fpi_usb_transfer_fill_bulk (img_trf, EP_IN, cls->data_buflen);
   img_trf->short_is_error = TRUE;
   fpi_usb_transfer_submit (g_steal_pointer (&img_trf), 0,
-                           priv->img_trf_cancel,
+                           priv->img_capture_cancel,
                            img_cb, NULL);
 }
 
+static void
+capture_reqs_cb (FpImageDevice *dev, GError *result, void *user_data)
+{
+  FpiDeviceAes3k *self = FPI_DEVICE_AES3K (dev);
+  FpiDeviceAes3kPrivate *priv = fpi_device_aes3k_get_instance_private (self);
+
+  if (result)
+    {
+      g_clear_object (&priv->img_capture_cancel);
+      fpi_image_device_session_error (dev, result);
+      return;
+    }
+
+  /* FIXME: we never cancel a pending capture. So we are likely leaving the
+   * hardware in a bad state should we abort the capture operation and the
+   * user does not touch the device.
+   * But, we don't know how we might cancel, so just leave it as is. */
+  do_capture (dev);
+}
+
+static void
+do_capture_start (FpImageDevice *dev)
+{
+  FpiDeviceAes3k *self = FPI_DEVICE_AES3K (dev);
+  FpiDeviceAes3kClass *cls = FPI_DEVICE_AES3K_GET_CLASS (self);
+
+  aes_write_regv (dev, cls->capture_reqs, cls->capture_reqs_len, capture_reqs_cb, NULL);
+}
+
 static void
 init_reqs_cb (FpImageDevice *dev, GError *result, void *user_data)
 {
-  FpiDeviceAes3kPrivate *priv = fpi_device_aes3k_get_instance_private (FPI_DEVICE_AES3K (dev));
-
   fpi_image_device_activate_complete (dev, result);
-  if (!result)
-    {
-      priv->img_trf_cancel = g_cancellable_new ();
-      do_capture (dev);
-    }
 }
 
 static void
 aes3k_dev_activate (FpImageDevice *dev)
 {
   FpiDeviceAes3k *self = FPI_DEVICE_AES3K (dev);
-  FpiDeviceAes3kPrivate *priv = fpi_device_aes3k_get_instance_private (self);
   FpiDeviceAes3kClass *cls = FPI_DEVICE_AES3K_GET_CLASS (self);
 
-  g_assert (!priv->img_trf_cancel);
   aes_write_regv (dev, cls->init_reqs, cls->init_reqs_len, init_reqs_cb, NULL);
 }
 
@@ -175,13 +198,28 @@ aes3k_dev_deactivate (FpImageDevice *dev)
   FpiDeviceAes3k *self = FPI_DEVICE_AES3K (dev);
   FpiDeviceAes3kPrivate *priv = fpi_device_aes3k_get_instance_private (self);
 
-  /* Deactivation finishes from the cancellation handler */
-  if (priv->img_trf_cancel)
-    g_cancellable_cancel (priv->img_trf_cancel);
+  /* If a capture is running, then deactivation finishes from the cancellation handler */
+  if (priv->img_capture_cancel)
+    g_cancellable_cancel (priv->img_capture_cancel);
   else
     fpi_image_device_deactivate_complete (dev, NULL);
 }
 
+static void
+aes3k_dev_change_state (FpImageDevice *dev, FpiImageDeviceState state)
+{
+  FpiDeviceAes3k *self = FPI_DEVICE_AES3K (dev);
+  FpiDeviceAes3kPrivate *priv = fpi_device_aes3k_get_instance_private (self);
+
+  if (state == FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON)
+    {
+      g_assert (!priv->img_capture_cancel);
+      priv->img_capture_cancel = g_cancellable_new ();
+
+      do_capture_start (dev);
+    }
+}
+
 static void
 fpi_device_aes3k_init (FpiDeviceAes3k *self)
 {
@@ -224,6 +262,7 @@ fpi_device_aes3k_class_init (FpiDeviceAes3kClass *klass)
   img_class->img_open = aes3k_dev_init;
   img_class->img_close = aes3k_dev_deinit;
   img_class->activate = aes3k_dev_activate;
+  img_class->change_state = aes3k_dev_change_state;
   img_class->deactivate = aes3k_dev_deactivate;
 
   /* Extremely low due to low image quality. */

libfprint/drivers/aes3k.h

@@ -57,4 +57,6 @@ struct _FpiDeviceAes3kClass
   gsize                data_buflen;    /* buffer length of usb bulk transfer */
   struct aes_regwrite *init_reqs;       /* initial values sent to device */
   gsize                init_reqs_len;
+  struct aes_regwrite *capture_reqs;       /* capture values sent to device */
+  gsize                capture_reqs_len;
 };

libfprint/drivers/aes4000.c

@@ -103,7 +103,9 @@ static struct aes_regwrite init_reqs[] = {
   { 0x9e, 0x53 },       /* clear challenge word bits */
   { 0x9f, 0x6b },       /* set some challenge word bits */
   { 0, 0 },
+};
 
+static struct aes_regwrite capture_reqs[] = {
   { 0x80, 0x00 },
   { 0x81, 0x00 },
   { 0, 0 },
@@ -152,4 +154,6 @@ fpi_device_aes4000_class_init (FpiDeviceAes4000Class *klass)
   aes_class->enlarge_factor = ENLARGE_FACTOR;
   aes_class->init_reqs = init_reqs;
   aes_class->init_reqs_len = G_N_ELEMENTS (init_reqs);
+  aes_class->capture_reqs = capture_reqs;
+  aes_class->capture_reqs_len = G_N_ELEMENTS (capture_reqs);
 }

libfprint/drivers/aesx660.c

@@ -54,6 +54,7 @@ static void complete_deactivation (FpImageDevice *dev);
 #define CALIBRATE_DATA_LEN 4
 #define FINGER_DET_DATA_LEN 4
 
+FP_GNUC_ACCESS (read_only, 3, 4)
 static void
 aesX660_send_cmd_timeout (FpiSsm                *ssm,
                           FpDevice              *_dev,
@@ -70,6 +71,7 @@ aesX660_send_cmd_timeout (FpiSsm                *ssm,
   fpi_usb_transfer_submit (transfer, timeout, NULL, callback, NULL);
 }
 
+FP_GNUC_ACCESS (read_only, 3, 4)
 static void
 aesX660_send_cmd (FpiSsm                *ssm,
                   FpDevice              *dev,
@@ -89,13 +91,12 @@ aesX660_read_response (FpiSsm                *ssm,
                        FpiUsbTransferCallback callback)
 {
   FpiUsbTransfer *transfer = fpi_usb_transfer_new (_dev);
-  unsigned char *data;
   GCancellable *cancel = NULL;
 
   if (cancellable)
     cancel = fpi_device_get_cancellable (_dev);
-  data = g_malloc (buf_len);
-  fpi_usb_transfer_fill_bulk_full (transfer, EP_IN, data, buf_len, NULL);
+
+  fpi_usb_transfer_fill_bulk (transfer, EP_IN, buf_len);
   transfer->ssm = ssm;
   transfer->short_is_error = short_is_error;
   fpi_usb_transfer_submit (transfer, BULK_TIMEOUT, cancel, callback, NULL);

libfprint/drivers/synaptics/synaptics.c

@@ -31,6 +31,9 @@ static const FpIdEntry id_table[] = {
   { .vid = SYNAPTICS_VENDOR_ID,  .pid = 0xBD,  },
   { .vid = SYNAPTICS_VENDOR_ID,  .pid = 0xE9,  },
   { .vid = SYNAPTICS_VENDOR_ID,  .pid = 0xDF,  },
+  { .vid = SYNAPTICS_VENDOR_ID,  .pid = 0xF9,  },
+  { .vid = SYNAPTICS_VENDOR_ID,  .pid = 0xFC,  },
+  { .vid = SYNAPTICS_VENDOR_ID,  .pid = 0xC2,  },
   { .vid = 0,  .pid = 0,  .driver_data = 0 },   /* terminating entry */
 };
 
@@ -659,6 +662,155 @@ verify (FpDevice *device)
   synaptics_sensor_cmd (self, 0, BMKT_CMD_VERIFY_USER, user_id, user_id_len, verify_msg_cb);
 }
 
+static void
+identify_complete_after_finger_removal (FpiDeviceSynaptics *self)
+{
+  FpDevice *device = FP_DEVICE (self);
+
+  if (self->finger_on_sensor)
+    {
+      fp_dbg ("delaying identify report until after finger removal!");
+      self->cmd_complete_on_removal = TRUE;
+    }
+  else
+    {
+      fpi_device_identify_complete (device, NULL);
+    }
+}
+
+
+static void
+identify_msg_cb (FpiDeviceSynaptics *self,
+                 bmkt_response_t    *resp,
+                 GError             *error)
+{
+  FpDevice *device = FP_DEVICE (self);
+
+  if (error)
+    {
+      fpi_device_identify_complete (device, error);
+      return;
+    }
+
+  if (resp == NULL && self->cmd_complete_on_removal)
+    {
+      fpi_device_identify_complete (device, NULL);
+      return;
+    }
+
+  g_assert (resp != NULL);
+
+  switch (resp->response_id)
+    {
+    case BMKT_RSP_ID_READY:
+      fp_info ("Place Finger on the Sensor!");
+      break;
+
+    case BMKT_RSP_ID_FAIL:
+      if (resp->result == BMKT_SENSOR_STIMULUS_ERROR)
+        {
+          fp_info ("Match error occurred");
+          fpi_device_identify_report (device, NULL, NULL,
+                                      fpi_device_retry_new (FP_DEVICE_RETRY_GENERAL));
+          identify_complete_after_finger_removal (self);
+        }
+      else if (resp->result == BMKT_FP_NO_MATCH)
+        {
+          fp_info ("Print didn't match");
+          fpi_device_identify_report (device, NULL, NULL, NULL);
+          identify_complete_after_finger_removal (self);
+        }
+      else if (resp->result == BMKT_FP_DATABASE_NO_RECORD_EXISTS)
+        {
+          fp_info ("Print is not in database");
+          fpi_device_identify_complete (device,
+                                        fpi_device_error_new (FP_DEVICE_ERROR_DATA_NOT_FOUND));
+        }
+      else
+        {
+          fp_warn ("identify has failed: %d", resp->result);
+          fpi_device_identify_complete (device,
+                                        fpi_device_error_new_msg (FP_DEVICE_ERROR_PROTO,
+                                                                  "Unexpected result from device %d",
+                                                                  resp->result));
+        }
+      break;
+
+    case BMKT_RSP_ID_OK:
+      {
+        FpPrint *print = NULL;
+        GPtrArray *prints = NULL;
+        g_autoptr(GVariant) data = NULL;
+        guint8 finger;
+        const guint8 *user_id;
+        gsize user_id_len = 0;
+        gint cnt = 0;
+        gboolean find = FALSE;
+
+        fpi_device_get_identify_data (device, &prints);
+
+        for (cnt = 0; cnt < prints->len; cnt++)
+          {
+            print = g_ptr_array_index (prints, cnt);
+            g_object_get (print, "fpi-data", &data, NULL);
+            g_debug ("data is %p", data);
+            parse_print_data (data, &finger, &user_id, &user_id_len);
+            if (user_id)
+              {
+                if (memcmp (resp->response.id_resp.user_id, user_id, user_id_len) == 0)
+                  {
+                    find = TRUE;
+                    break;
+                  }
+              }
+          }
+        if(find)
+          {
+            fpi_device_identify_report (device, print, print, NULL);
+            fpi_device_identify_complete (device, NULL);
+          }
+        else
+          {
+            fpi_device_identify_report (device, NULL, NULL, NULL);
+            identify_complete_after_finger_removal (self);
+          }
+      }
+    }
+}
+
+static void
+identify (FpDevice *device)
+{
+  FpiDeviceSynaptics *self = FPI_DEVICE_SYNAPTICS (device);
+  FpPrint *print = NULL;
+  GPtrArray *prints = NULL;
+
+  g_autoptr(GVariant) data = NULL;
+  guint8 finger;
+  const guint8 *user_id;
+  gsize user_id_len = 0;
+  gint cnt = 0;
+
+  fpi_device_get_identify_data (device, &prints);
+
+  for (cnt = 0; cnt < prints->len; cnt++)
+    {
+      print = g_ptr_array_index (prints, cnt);
+      g_object_get (print, "fpi-data", &data, NULL);
+      g_debug ("data is %p", data);
+      if (!parse_print_data (data, &finger, &user_id, &user_id_len))
+        {
+          fpi_device_identify_complete (device,
+                                        fpi_device_error_new (FP_DEVICE_ERROR_DATA_INVALID));
+          return;
+        }
+    }
+  G_DEBUG_HERE ();
+
+  synaptics_sensor_cmd (self, 0, BMKT_CMD_ID_USER, NULL, 0, identify_msg_cb);
+}
+
+
 static void
 enroll_msg_cb (FpiDeviceSynaptics *self,
                bmkt_response_t    *resp,
@@ -1152,6 +1304,7 @@ fpi_device_synaptics_class_init (FpiDeviceSynapticsClass *klass)
   dev_class->close = dev_exit;
   dev_class->probe = dev_probe;
   dev_class->verify = verify;
+  dev_class->identify = identify;
   dev_class->enroll = enroll;
   dev_class->delete = delete_print;
   dev_class->cancel = cancel;

libfprint/drivers/upektc_img.c

@@ -78,6 +78,7 @@ upektc_img_cmd_update_crc (unsigned char *cmd_buf, size_t size)
   cmd_buf[size - 1] = (crc & 0xff00) >> 8;
 }
 
+FP_GNUC_ACCESS (read_only, 3, 4)
 static void
 upektc_img_submit_req (FpiSsm                *ssm,
                        FpImageDevice         *dev,

libfprint/drivers/vfs0050.c

@@ -42,6 +42,7 @@ async_write_callback (FpiUsbTransfer *transfer, FpDevice *device,
 }
 
 /* Send data to EP1, the only out endpoint */
+FP_GNUC_ACCESS (read_only, 3, 4)
 static void
 async_write (FpiSsm   *ssm,
              FpDevice *dev,

libfprint/drivers/vfs301_proto.c

@@ -93,6 +93,7 @@ usb_recv (FpDeviceVfs301 *dev, guint8 endpoint, int max_bytes, FpiUsbTransfer **
     *out = g_steal_pointer (&transfer);
 }
 
+FP_GNUC_ACCESS (read_only, 2, 3)
 static void
 usb_send (FpDeviceVfs301 *dev, const guint8 *data, gssize length, GError **error)
 {
@@ -508,30 +509,30 @@ vfs301_proto_process_event_cb (FpiUsbTransfer *transfer,
                                FpDevice *device,
                                gpointer user_data, GError *error)
 {
-  FpDeviceVfs301 *dev = user_data;
+  FpDeviceVfs301 *self = FPI_DEVICE_VFS301 (device);
 
   if (error)
     {
       g_warning ("Error receiving data: %s", error->message);
       g_error_free (error);
-      dev->recv_progress = VFS301_FAILURE;
+      self->recv_progress = VFS301_FAILURE;
       return;
     }
   else if (transfer->actual_length < transfer->length)
     {
       /* TODO: process the data anyway? */
-      dev->recv_progress = VFS301_ENDED;
+      self->recv_progress = VFS301_ENDED;
       return;
     }
   else
     {
       FpiUsbTransfer *new;
-      if (!vfs301_proto_process_data (dev,
+      if (!vfs301_proto_process_data (self,
                                       transfer->length == VFS301_FP_RECV_LEN_1,
                                       transfer->buffer,
                                       transfer->actual_length))
         {
-          dev->recv_progress = VFS301_ENDED;
+          self->recv_progress = VFS301_ENDED;
           return;
         }
 

libfprint/drivers/virtual-image.c

@@ -42,6 +42,7 @@ struct _FpDeviceVirtualImage
 
   GSocketListener   *listener;
   GSocketConnection *connection;
+  GCancellable      *listen_cancellable;
   GCancellable      *cancellable;
 
   gint               socket_fd;
@@ -185,15 +186,25 @@ recv_image_hdr_recv_cb (GObject      *source_object,
 }
 
 static void
-recv_image (FpDeviceVirtualImage *dev, GInputStream *stream)
+recv_image (FpDeviceVirtualImage *self, GInputStream *stream)
 {
-  g_input_stream_read_all_async (stream,
-                                 dev->recv_img_hdr,
-                                 sizeof (dev->recv_img_hdr),
-                                 G_PRIORITY_DEFAULT,
-                                 dev->cancellable,
-                                 recv_image_hdr_recv_cb,
-                                 dev);
+  FpiImageDeviceState state;
+
+  g_object_get (self, "fpi-image-device-state", &state, NULL);
+
+  g_debug ("Starting image receive (if active), state is: %i", state);
+
+  /* Only register if the state is active. */
+  if (state != FPI_IMAGE_DEVICE_STATE_INACTIVE)
+    {
+      g_input_stream_read_all_async (stream,
+                                     self->recv_img_hdr,
+                                     sizeof (self->recv_img_hdr),
+                                     G_PRIORITY_DEFAULT,
+                                     self->cancellable,
+                                     recv_image_hdr_recv_cb,
+                                     self);
+    }
 }
 
 static void
@@ -217,11 +228,23 @@ new_connection_cb (GObject *source_object, GAsyncResult *res, gpointer user_data
       start_listen (dev);
     }
 
-  /* Always further connections (but we disconnect them immediately
+  /* Always accept further connections (but we disconnect them immediately
    * if we already have a connection). */
   start_listen (dev);
+
   if (dev->connection)
     {
+      /* We may not have noticed that the stream was closed,
+       * if the device is deactivated. Double check here. */
+      g_input_stream_is_closed (g_io_stream_get_input_stream (G_IO_STREAM (dev->connection)));
+
+      g_io_stream_close (G_IO_STREAM (dev->connection), NULL, NULL);
+      g_clear_object (&dev->connection);
+    }
+
+  if (dev->connection)
+    {
+      g_warning ("Rejecting new connection");
       g_io_stream_close (G_IO_STREAM (connection), NULL, NULL);
       g_object_unref (connection);
       return;
@@ -231,16 +254,15 @@ new_connection_cb (GObject *source_object, GAsyncResult *res, gpointer user_data
   dev->automatic_finger = TRUE;
   stream = g_io_stream_get_input_stream (G_IO_STREAM (connection));
 
-  recv_image (dev, stream);
-
   fp_dbg ("Got a new connection!");
+  recv_image (dev, stream);
 }
 
 static void
 start_listen (FpDeviceVirtualImage *dev)
 {
   g_socket_listener_accept_async (dev->listener,
-                                  dev->cancellable,
+                                  dev->listen_cancellable,
                                   new_connection_cb,
                                   dev);
 }
@@ -285,6 +307,7 @@ dev_init (FpImageDevice *dev)
 
   self->listener = g_steal_pointer (&listener);
   self->cancellable = g_cancellable_new ();
+  self->listen_cancellable = g_cancellable_new ();
 
   start_listen (self);
 
@@ -300,7 +323,9 @@ dev_deinit (FpImageDevice *dev)
   G_DEBUG_HERE ();
 
   g_cancellable_cancel (self->cancellable);
+  g_cancellable_cancel (self->listen_cancellable);
   g_clear_object (&self->cancellable);
+  g_clear_object (&self->listen_cancellable);
   g_clear_object (&self->listener);
   g_clear_object (&self->connection);
 
@@ -308,6 +333,30 @@ dev_deinit (FpImageDevice *dev)
   fpi_device_add_timeout (FP_DEVICE (dev), 100, (FpTimeoutFunc) fpi_image_device_close_complete, NULL, NULL);
 }
 
+static void
+dev_activate (FpImageDevice *dev)
+{
+  FpDeviceVirtualImage *self = FPI_DEVICE_VIRTUAL_IMAGE (dev);
+
+  if (self->connection)
+    recv_image (self, g_io_stream_get_input_stream (G_IO_STREAM (self->connection)));
+
+  fpi_image_device_activate_complete (dev, NULL);
+}
+
+static void
+dev_deactivate (FpImageDevice *dev)
+{
+  FpDeviceVirtualImage *self = FPI_DEVICE_VIRTUAL_IMAGE (dev);
+
+  g_cancellable_cancel (self->cancellable);
+  g_clear_object (&self->cancellable);
+  self->cancellable = g_cancellable_new ();
+
+  /* XXX: Need to wait for the operation to be cancelled. */
+  fpi_device_add_timeout (FP_DEVICE (dev), 10, (FpTimeoutFunc) fpi_image_device_deactivate_complete, NULL, NULL);
+}
+
 static void
 fpi_device_virtual_image_init (FpDeviceVirtualImage *self)
 {
@@ -331,4 +380,7 @@ fpi_device_virtual_image_class_init (FpDeviceVirtualImageClass *klass)
 
   img_class->img_open = dev_init;
   img_class->img_close = dev_deinit;
+
+  img_class->activate = dev_activate;
+  img_class->deactivate = dev_deactivate;
 }

libfprint/fp-image-device-private.h

@@ -29,11 +29,13 @@ typedef struct
   gboolean            active;
   gboolean            cancelling;
 
-  gboolean            enroll_await_on_pending;
+  gboolean            finger_present;
+
   gint                enroll_stage;
 
-  guint               pending_activation_timeout_id;
-  gboolean            pending_activation_timeout_waiting_finger_off;
+  gboolean            minutiae_scan_active;
+  GError             *action_error;
+  FpImage            *capture_image;
 
   gint                bz3_threshold;
 } FpImageDevicePrivate;

libfprint/fp-image-device.c

@@ -56,44 +56,6 @@ static guint signals[LAST_SIGNAL] = { 0 };
  * - sanitize_image seems a bit odd, in particular the sizing stuff.
  **/
 
-/* Static helper functions */
-
-static gboolean
-pending_activation_timeout (gpointer user_data)
-{
-  FpImageDevice *self = FP_IMAGE_DEVICE (user_data);
-  FpDevice *device = FP_DEVICE (self);
-  FpImageDevicePrivate *priv = fp_image_device_get_instance_private (self);
-  FpiDeviceAction action = fpi_device_get_current_action (device);
-  GError *error;
-
-  priv->pending_activation_timeout_id = 0;
-
-  if (priv->pending_activation_timeout_waiting_finger_off)
-    error = fpi_device_retry_new_msg (FP_DEVICE_RETRY_REMOVE_FINGER,
-                                      "Remove finger before requesting another scan operation");
-  else
-    error = fpi_device_retry_new (FP_DEVICE_RETRY_GENERAL);
-
-  if (action == FPI_DEVICE_ACTION_VERIFY)
-    {
-      fpi_device_verify_report (device, FPI_MATCH_ERROR, NULL, error);
-      fpi_device_verify_complete (device, NULL);
-    }
-  else if (action == FPI_DEVICE_ACTION_IDENTIFY)
-    {
-      fpi_device_identify_report (device, NULL, NULL, error);
-      fpi_device_identify_complete (device, NULL);
-    }
-  else
-    {
-      /* Can this happen for enroll? */
-      fpi_device_action_error (device, error);
-    }
-
-  return G_SOURCE_REMOVE;
-}
-
 /* Callbacks/vfuncs */
 static void
 fp_image_device_open (FpDevice *device)
@@ -112,19 +74,8 @@ fp_image_device_close (FpDevice *device)
   FpImageDeviceClass *cls = FP_IMAGE_DEVICE_GET_CLASS (self);
   FpImageDevicePrivate *priv = fp_image_device_get_instance_private (self);
 
-  /* In the close case we may need to wait/force deactivation first.
-   * Three possible cases:
-   *  1. We are inactive
-   *     -> immediately close
-   *  2. We are waiting for finger off
-   *     -> immediately deactivate
-   *  3. We are deactivating
-   *     -> handled by deactivate_complete */
-
-  if (!priv->active)
-    cls->img_close (self);
-  else if (priv->state != FPI_IMAGE_DEVICE_STATE_INACTIVE)
-    fpi_image_device_deactivate (self);
+  g_assert (priv->active == FALSE);
+  cls->img_close (self);
 }
 
 static void
@@ -146,12 +97,6 @@ fp_image_device_cancel_action (FpDevice *device)
       priv->cancelling = TRUE;
       fpi_image_device_deactivate (self);
       priv->cancelling = FALSE;
-
-      /* XXX: Some nicer way of doing this would be good. */
-      fpi_device_action_error (FP_DEVICE (self),
-                               g_error_new (G_IO_ERROR,
-                                            G_IO_ERROR_CANCELLED,
-                                            "Device operation was cancelled"));
     }
 }
 
@@ -188,27 +133,9 @@ fp_image_device_start_capture_action (FpDevice *device)
     }
 
   priv->enroll_stage = 0;
-  priv->enroll_await_on_pending = FALSE;
-
-  /* The device might still be deactivating from a previous call.
-   * In that situation, try to wait for a bit before reporting back an
-   * error (which will usually say that the user should remove the
-   * finger).
-   */
-  if (priv->state != FPI_IMAGE_DEVICE_STATE_INACTIVE || priv->active)
-    {
-      g_debug ("Got a new request while the device was still active");
-      g_assert (priv->pending_activation_timeout_id == 0);
-      priv->pending_activation_timeout_id =
-        g_timeout_add (100, pending_activation_timeout, device);
-
-      if (priv->state == FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF)
-        priv->pending_activation_timeout_waiting_finger_off = TRUE;
-      else
-        priv->pending_activation_timeout_waiting_finger_off = FALSE;
-
-      return;
-    }
+  /* The internal state machine guarantees both of these. */
+  g_assert (!priv->finger_present);
+  g_assert (!priv->minutiae_scan_active);
 
   /* And activate the device; we rely on fpi_image_device_activate_complete()
    * to be called when done (or immediately). */
@@ -225,7 +152,6 @@ fp_image_device_finalize (GObject *object)
   FpImageDevicePrivate *priv = fp_image_device_get_instance_private (self);
 
   g_assert (priv->active == FALSE);
-  g_clear_handle_id (&priv->pending_activation_timeout_id, g_source_remove);
 
   G_OBJECT_CLASS (fp_image_device_parent_class)->finalize (object);
 }

libfprint/fpi-compat.h

@@ -37,3 +37,9 @@ typedef struct _FpDeviceClass FpDeviceClass;
 G_DEFINE_AUTOPTR_CLEANUP_FUNC (FpDeviceClass, g_type_class_unref);
 G_DEFINE_AUTOPTR_CLEANUP_FUNC (GDate, g_date_free);
 #endif
+
+#if  __GNUC__ > 10 || (__GNUC__ == 10 && __GNUC_MINOR__ >= 1)
+#define FP_GNUC_ACCESS(m, p, s) __attribute__((access (m, p, s)))
+#else
+#define FP_GNUC_ACCESS(m, p, s)
+#endif

libfprint/fpi-image-device.c

@@ -56,10 +56,6 @@ fpi_image_device_activate (FpImageDevice *self)
   priv->state = FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON;
   g_object_notify (G_OBJECT (self), "fpi-image-device-state");
 
-  /* We might have been waiting for deactivation to finish before
-   * starting the next operation. */
-  g_clear_handle_id (&priv->pending_activation_timeout_id, g_source_remove);
-
   fp_dbg ("Activating image device");
   cls->activate (self);
 }
@@ -100,10 +96,6 @@ fp_image_device_change_state (FpImageDevice *self, FpiImageDeviceState state)
   /* Cannot change to inactive using this function. */
   g_assert (state != FPI_IMAGE_DEVICE_STATE_INACTIVE);
 
-  /* We might have been waiting for the finger to go OFF to start the
-   * next operation. */
-  g_clear_handle_id (&priv->pending_activation_timeout_id, g_source_remove);
-
   prev_state_str = g_enum_to_string (FPI_TYPE_IMAGE_DEVICE_STATE, priv->state);
   state_str = g_enum_to_string (FPI_TYPE_IMAGE_DEVICE_STATE, state);
   fp_dbg ("Image device internal state change from %s to %s",
@@ -119,15 +111,75 @@ fp_image_device_enroll_maybe_await_finger_on (FpImageDevice *self)
 {
   FpImageDevicePrivate *priv = fp_image_device_get_instance_private (self);
 
-  if (priv->enroll_await_on_pending)
+  /* We wait for both the minutiae scan to complete and the finger to
+   * be removed before we switch to AWAIT_FINGER_ON. */
+  if (priv->minutiae_scan_active || priv->finger_present)
+    return;
+
+  fp_image_device_change_state (self, FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON);
+}
+
+static void
+fp_image_device_maybe_complete_action (FpImageDevice *self, GError *error)
+{
+  FpImageDevicePrivate *priv = fp_image_device_get_instance_private (self);
+  FpDevice *device = FP_DEVICE (self);
+  FpiDeviceAction action;
+
+  if (error)
     {
-      priv->enroll_await_on_pending = FALSE;
-      fp_image_device_change_state (self, FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON);
+      /* Keep the first error we encountered, but not if it is of type retry */
+      if (priv->action_error && !(priv->action_error->domain == FP_DEVICE_RETRY))
+        {
+          g_warning ("Will complete with first error, new error was: %s", error->message);
+          g_free (error);
+        }
+      else
+        {
+          g_clear_error (&priv->action_error);
+          priv->action_error = error;
+        }
+    }
+
+  /* Do not complete if the device is still active or a minutiae scan is pending. */
+  if (priv->active || priv->minutiae_scan_active)
+    return;
+
+  if (!priv->action_error)
+    g_cancellable_set_error_if_cancelled (fpi_device_get_cancellable (device), &priv->action_error);
+
+  if (priv->action_error)
+    {
+      fpi_device_action_error (device, g_steal_pointer (&priv->action_error));
+      g_clear_object (&priv->capture_image);
+      return;
+    }
+
+  /* We are done, report the result. */
+  action = fpi_device_get_current_action (FP_DEVICE (self));
+
+  if (action == FPI_DEVICE_ACTION_ENROLL)
+    {
+      FpPrint *enroll_print;
+      fpi_device_get_enroll_data (device, &enroll_print);
+
+      fpi_device_enroll_complete (device, g_object_ref (enroll_print), NULL);
+    }
+  else if (action == FPI_DEVICE_ACTION_VERIFY)
+    {
+      fpi_device_verify_complete (device, NULL);
+    }
+  else if (action == FPI_DEVICE_ACTION_IDENTIFY)
+    {
+      fpi_device_identify_complete (device, NULL);
+    }
+  else if (action == FPI_DEVICE_ACTION_CAPTURE)
+    {
+      fpi_device_capture_complete (device, g_steal_pointer (&priv->capture_image), NULL);
     }
   else
     {
-      fp_dbg ("Awaiting finger on");
-      priv->enroll_await_on_pending = TRUE;
+      g_assert_not_reached ();
     }
 }
 
@@ -143,13 +195,15 @@ fpi_image_device_minutiae_detected (GObject *source_object, GAsyncResult *res, g
   FpiDeviceAction action;
 
   /* Note: We rely on the device to not disappear during an operation. */
+  priv = fp_image_device_get_instance_private (FP_IMAGE_DEVICE (device));
+  priv->minutiae_scan_active = FALSE;
 
   if (!fp_image_detect_minutiae_finish (image, res, &error))
     {
       /* Cancel operation . */
       if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
         {
-          fpi_device_action_error (device, g_steal_pointer (&error));
+          fp_image_device_maybe_complete_action (self, g_steal_pointer (&error));
           fpi_image_device_deactivate (self);
           return;
         }
@@ -161,13 +215,12 @@ fpi_image_device_minutiae_detected (GObject *source_object, GAsyncResult *res, g
       error = fpi_device_retry_new_msg (FP_DEVICE_RETRY_GENERAL, "Minutiae detection failed, please retry");
     }
 
-  priv = fp_image_device_get_instance_private (FP_IMAGE_DEVICE (device));
   action = fpi_device_get_current_action (device);
 
   if (action == FPI_DEVICE_ACTION_CAPTURE)
     {
-      fpi_device_capture_complete (device, g_steal_pointer (&image), error);
-      fpi_image_device_deactivate (self);
+      priv->capture_image = g_steal_pointer (&image);
+      fp_image_device_maybe_complete_action (self, g_steal_pointer (&error));
       return;
     }
 
@@ -181,7 +234,8 @@ fpi_image_device_minutiae_detected (GObject *source_object, GAsyncResult *res, g
 
           if (error->domain != FP_DEVICE_RETRY)
             {
-              fpi_device_action_error (device, error);
+              fp_image_device_maybe_complete_action (self, g_steal_pointer (&error));
+              /* We might not yet be deactivating, if we are enrolling. */
               fpi_image_device_deactivate (self);
               return;
             }
@@ -205,7 +259,7 @@ fpi_image_device_minutiae_detected (GObject *source_object, GAsyncResult *res, g
       /* Start another scan or deactivate. */
       if (priv->enroll_stage == IMG_ENROLL_STAGES)
         {
-          fpi_device_enroll_complete (device, g_object_ref (enroll_print), NULL);
+          fp_image_device_maybe_complete_action (self, g_steal_pointer (&error));
           fpi_image_device_deactivate (self);
         }
       else
@@ -226,8 +280,8 @@ fpi_image_device_minutiae_detected (GObject *source_object, GAsyncResult *res, g
 
       if (!error || error->domain == FP_DEVICE_RETRY)
         fpi_device_verify_report (device, result, g_steal_pointer (&print), g_steal_pointer (&error));
-      fpi_device_verify_complete (device, error);
-      fpi_image_device_deactivate (self);
+
+      fp_image_device_maybe_complete_action (self, g_steal_pointer (&error));
     }
   else if (action == FPI_DEVICE_ACTION_IDENTIFY)
     {
@@ -249,8 +303,8 @@ fpi_image_device_minutiae_detected (GObject *source_object, GAsyncResult *res, g
 
       if (!error || error->domain == FP_DEVICE_RETRY)
         fpi_device_identify_report (device, result, g_steal_pointer (&print), g_steal_pointer (&error));
-      fpi_device_identify_complete (device, error);
-      fpi_image_device_deactivate (self);
+
+      fp_image_device_maybe_complete_action (self, g_steal_pointer (&error));
     }
   else
     {
@@ -323,24 +377,18 @@ fpi_image_device_report_finger_status (FpImageDevice *self,
 
   g_debug ("Image device reported finger status: %s", present ? "on" : "off");
 
+  priv->finger_present = present;
+
   if (present && priv->state == FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON)
     {
       fp_image_device_change_state (self, FPI_IMAGE_DEVICE_STATE_CAPTURE);
     }
   else if (!present && priv->state == FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF)
     {
-      /* We need to deactivate or continue to await finger */
-
-      /* There are three possible situations:
-       *  1. We are deactivating the device and the action is still in progress
-       *     (minutiae detection).
-       *  2. We are still deactivating the device after an action completed
-       *  3. We were waiting for finger removal to start the new action
-       * Either way, we always end up deactivating except for the enroll case.
+      /* If we are in the non-enroll case, we always deactivate.
        *
-       * The enroll case is special as AWAIT_FINGER_ON should only happen after
-       * minutiae detection to prevent deactivation (without cancellation)
-       * from the AWAIT_FINGER_ON state.
+       * In the enroll case, the decision can only be made after minutiae
+       * detection has finished.
        */
       if (action != FPI_DEVICE_ACTION_ENROLL)
         fpi_image_device_deactivate (self);
@@ -378,16 +426,18 @@ fpi_image_device_image_captured (FpImageDevice *self, FpImage *image)
                     action == FPI_DEVICE_ACTION_IDENTIFY ||
                     action == FPI_DEVICE_ACTION_CAPTURE);
 
-  fp_image_device_change_state (self, FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF);
-
   g_debug ("Image device captured an image");
 
+  priv->minutiae_scan_active = TRUE;
+
   /* XXX: We also detect minutiae in capture mode, we solely do this
    *      to normalize the image which will happen as a by-product. */
   fp_image_detect_minutiae (image,
                             fpi_device_get_cancellable (FP_DEVICE (self)),
                             fpi_image_device_minutiae_detected,
                             self);
+
+  fp_image_device_change_state (self, FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF);
 }
 
 /**
@@ -423,36 +473,33 @@ fpi_image_device_retry_scan (FpImageDevice *self, FpDeviceRetry retry)
       g_debug ("Reporting retry during enroll");
       fpi_device_enroll_progress (FP_DEVICE (self), priv->enroll_stage, NULL, error);
 
-      /* Wait for finger removal and re-touch.
-       * TODO: Do we need to check that the finger is already off? */
-      priv->enroll_await_on_pending = TRUE;
       fp_image_device_change_state (self, FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF);
     }
   else if (action == FPI_DEVICE_ACTION_VERIFY)
     {
       fpi_device_verify_report (FP_DEVICE (self), FPI_MATCH_ERROR, NULL, error);
+      fp_image_device_maybe_complete_action (self, NULL);
       priv->cancelling = TRUE;
       fpi_image_device_deactivate (self);
       priv->cancelling = FALSE;
-      fpi_device_verify_complete (FP_DEVICE (self), NULL);
+
     }
   else if (action == FPI_DEVICE_ACTION_IDENTIFY)
     {
       fpi_device_identify_report (FP_DEVICE (self), NULL, NULL, error);
+      fp_image_device_maybe_complete_action (self, NULL);
       priv->cancelling = TRUE;
       fpi_image_device_deactivate (self);
       priv->cancelling = FALSE;
-      fpi_device_identify_complete (FP_DEVICE (self), NULL);
     }
   else
     {
-      /* We abort the operation and let the surrounding code retry in the
-       * non-enroll case (this is identical to a session error). */
-      g_debug ("Abort current operation due to retry (non-enroll case)");
+      /* The capture case where there is no early reporting. */
+      g_debug ("Abort current operation due to retry (no early-reporting)");
+      fp_image_device_maybe_complete_action (self, error);
       priv->cancelling = TRUE;
       fpi_image_device_deactivate (self);
       priv->cancelling = FALSE;
-      fpi_device_action_error (FP_DEVICE (self), error);
     }
 }
 
@@ -512,9 +559,9 @@ fpi_image_device_session_error (FpImageDevice *self, GError *error)
     g_warning ("Driver should report retries using fpi_image_device_retry_scan!");
 
   priv->cancelling = TRUE;
+  fp_image_device_maybe_complete_action (self, error);
   fpi_image_device_deactivate (self);
   priv->cancelling = FALSE;
-  fpi_device_action_error (FP_DEVICE (self), error);
 }
 
 /**
@@ -565,8 +612,6 @@ void
 fpi_image_device_deactivate_complete (FpImageDevice *self, GError *error)
 {
   FpImageDevicePrivate *priv = fp_image_device_get_instance_private (self);
-  FpImageDeviceClass *cls = FP_IMAGE_DEVICE_GET_CLASS (self);
-  FpiDeviceAction action;
 
   g_return_if_fail (priv->active == TRUE);
   g_return_if_fail (priv->state == FPI_IMAGE_DEVICE_STATE_INACTIVE);
@@ -575,26 +620,10 @@ fpi_image_device_deactivate_complete (FpImageDevice *self, GError *error)
 
   priv->active = FALSE;
 
-  /* Deactivation completed. As we deactivate in the background
-   * there may already be a new task pending. Check whether we
-   * need to do anything. */
-  action = fpi_device_get_current_action (FP_DEVICE (self));
+  /* Assume finger was removed. */
+  priv->finger_present = FALSE;
 
-  /* Special case, if we should be closing, but didn't due to a running
-   * deactivation, then do so now. */
-  if (action == FPI_DEVICE_ACTION_CLOSE)
-    {
-      cls->img_close (self);
-      return;
-    }
-
-  /* We might be waiting to be able to activate again. */
-  if (priv->pending_activation_timeout_id)
-    {
-      g_clear_handle_id (&priv->pending_activation_timeout_id, g_source_remove);
-      priv->pending_activation_timeout_id =
-        g_idle_add ((GSourceFunc) fpi_image_device_activate, self);
-    }
+  fp_image_device_maybe_complete_action (self, error);
 }
 
 /**

libfprint/fpi-usb-transfer.c

@@ -354,6 +354,24 @@ transfer_finish_cb (GObject *source_object, GAsyncResult *res, gpointer user_dat
   fpi_usb_transfer_unref (transfer);
 }
 
+static gboolean
+transfer_cancel_cb (FpiUsbTransfer *transfer)
+{
+  GError *error;
+  FpiUsbTransferCallback callback;
+
+  error = g_error_new_literal (G_IO_ERROR,
+                               G_IO_ERROR_CANCELLED,
+                               "Transfer was cancelled before being started");
+  callback = transfer->callback;
+  transfer->callback = NULL;
+  transfer->actual_length = -1;
+  callback (transfer, transfer->device, transfer->user_data, error);
+
+  fpi_usb_transfer_unref (transfer);
+
+  return G_SOURCE_REMOVE;
+}
 
 /**
  * fpi_usb_transfer_submit:
@@ -387,6 +405,18 @@ fpi_usb_transfer_submit (FpiUsbTransfer        *transfer,
 
   log_transfer (transfer, TRUE, NULL);
 
+  /* Work around libgusb cancellation issue, see
+   *   https://github.com/hughsie/libgusb/pull/42
+   * should be fixed with libgusb 0.3.7.
+   * Note that this is not race free, we rely on libfprint and API users
+   * not cancelling from a different thread here.
+   */
+  if (cancellable && g_cancellable_is_cancelled (cancellable))
+    {
+      g_idle_add ((GSourceFunc) transfer_cancel_cb, transfer);
+      return;
+    }
+
   switch (transfer->type)
     {
     case FP_TRANSFER_BULK:

libfprint/fpi-usb-transfer.h

@@ -20,6 +20,7 @@
 #pragma once
 
 #include <gusb.h>
+#include "fpi-compat.h"
 #include "fpi-device.h"
 
 G_BEGIN_DECLS
@@ -115,6 +116,7 @@ void               fpi_usb_transfer_fill_bulk (FpiUsbTransfer *transfer,
                                                guint8          endpoint,
                                                gsize           length);
 
+FP_GNUC_ACCESS (read_only, 3, 4)
 void               fpi_usb_transfer_fill_bulk_full (FpiUsbTransfer *transfer,
                                                     guint8          endpoint,
                                                     guint8         *buffer,
@@ -134,6 +136,7 @@ void               fpi_usb_transfer_fill_interrupt (FpiUsbTransfer *transfer,
                                                     guint8          endpoint,
                                                     gsize           length);
 
+FP_GNUC_ACCESS (read_only, 3, 4)
 void               fpi_usb_transfer_fill_interrupt_full (FpiUsbTransfer *transfer,
                                                          guint8          endpoint,
                                                          guint8         *buffer,

tests/README-umockdev

@@ -1,53 +0,0 @@
-To create a new umockdev test, you should:
-
- 1. Decide on what to test, the easiest case is just using the existing
-    capture test case.
- 2. Find the USB device you are testing with lsusb, e.g.:
-     Bus 001 Device 005: ID 138a:0090 Validity Sensors, Inc. VFS7500 Touch Fingerprint Sensor
-    This means we need to record USB device /dev/bus/usb/001/005
- 3. Run "umockdev-record /dev/bus/usb/001/005 >device"
-    This records the information about device, it should be placed into test/DRIVER/device
- 4. Run the test, for a capture test this would be:
-      umockdev-record -i /dev/bus/usb/001/005=capture.ioctl -- ./capture.py capture.png
-    This will create a capture.ioctl and capture.png file.
-    Please set the FP_DEVICE_EMULATION=1 environment variable. You may need
-    to adjust the driver to adapt to the emulated environment (mainly if it
-    uses random numbers, see synaptics.c for an example).
- 5. Place all files into the driver subdirectory test/DRIVER,
-    i.e. device, capture.ioctl, capture.png
- 6. Add glue to meson.build
- 7. Test whether everything works as expected
-
-Please note, there is no need to use a real finger print in this case. If
-you would like to avoid submitting your own fingerprint then please just
-use e.g. the side of your finger, arm, or anything else that will produce
-an image with the device.
-
-
-Note that umockdev-record groups URBs aggressively. In most cases, manual
-intervention is unfortunately required. In most cases, drivers do a chain
-of commands like e.g. A then B each with a different reply. Umockdev will
-create a file like:
-
-A
- reply 1
- reply 2
-B
- reply 1
- reply 2
-
-which then needs to be re-ordered to be:
-
-A
- reply 1
-B
- reply 1
-A
- reply 2
-B
- reply 2
-
-Other changes may be needed to get everything working. For example the elan
-driver relies on a timeout that is not reported correctly. In this case the
-driver works around it by interpreting the protocol error differently in
-the virtual environment.