scd: Fix SERIALNO for multiple devices.

* scd/app.c (select_application): Fix the logic if periodical check is
needed.  If it is needed for newly found device(s), kick the loop.
(scd_update_reader_status_file): Return value if select(2) should be
called with timeout.
* scd/ccid-driver.c (ccid_require_get_status): Don't return 0 for
token with no interrupt transfer for now.
* scd/command.c (open_card_with_request): Fix scan by SERIALNO.
* scd/scdaemon.c (update_usb): Remove.
(handle_connections): Evaluate need_tick after handle_tick.

Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>

scd/app.c

@@ -333,6 +333,7 @@ select_application (ctrl_t ctrl, const char *name, app_t *r_app,
       struct dev_list *l;
       int periodical_check_needed = 0;
 
+      /* Scan the devices to find new device(s).  */
       err = apdu_dev_list_start (opt.reader_port, &l);
       if (err)
         return err;
@@ -340,14 +341,14 @@ select_application (ctrl_t ctrl, const char *name, app_t *r_app,
       while (1)
         {
           int slot;
-          int periodical_check_needed;
+          int periodical_check_needed_this;
 
           slot = apdu_open_reader (l);
           if (slot < 0)
             break;
 
-          periodical_check_needed = apdu_connect (slot);
+          periodical_check_needed_this = apdu_connect (slot);
-          if (periodical_check_needed < 0)
+          if (periodical_check_needed_this < 0)
             {
               /* We close a reader with no card.  */
               err = gpg_error (GPG_ERR_ENODEV);
@@ -355,8 +356,8 @@ select_application (ctrl_t ctrl, const char *name, app_t *r_app,
           else
             {
               err = app_new_register (slot, ctrl, name,
-                                      periodical_check_needed);
+                                      periodical_check_needed_this);
-              if (periodical_check_needed)
+              if (periodical_check_needed_this)
                 periodical_check_needed = 1;
             }
 
@@ -365,7 +366,11 @@ select_application (ctrl_t ctrl, const char *name, app_t *r_app,
         }
 
       apdu_dev_list_finish (l);
-      update_usb (periodical_check_needed);
+
+      /* If periodical check is needed for new device(s), kick the
+       scdaemon loop.  */
+      if (periodical_check_needed)
+        scd_kick_the_loop ();
     }
 
   npth_mutex_lock (&app_list_lock);
@@ -1011,12 +1016,11 @@ report_change (int slot, int old_status, int cur_status)
   xfree (homestr);
 }
 
-void
+int
 scd_update_reader_status_file (void)
 {
   app_t a, app_next;
   int periodical_check_needed = 0;
-  int removal_detected = 0;
 
   npth_mutex_lock (&app_list_lock);
   for (a = app_top; a; a = app_next)
@@ -1050,7 +1054,6 @@ scd_update_reader_status_file (void)
               log_debug ("Removal of a card: %d\n", a->slot);
               apdu_close_reader (a->slot);
               deallocate_app (a);
-              removal_detected = 1;
             }
           else
             {
@@ -1067,8 +1070,7 @@ scd_update_reader_status_file (void)
     }
   npth_mutex_unlock (&app_list_lock);
 
-  if (removal_detected)
+  return periodical_check_needed;
-    update_usb (periodical_check_needed);
 }
 
 /* This function must be called once to initialize this module.  This

scd/ccid-driver.c

@@ -2073,17 +2073,28 @@ ccid_open_reader (const char *spec_reader_name, int idx,
 int
 ccid_require_get_status (ccid_driver_t handle)
 {
+  /* When a card reader supports interrupt transfer to check the
+     status of card, it is possible to submit only an interrupt
+     transfer, and no check is required by application layer.  USB can
+     detect removal of a card and can detect removal of a reader.
+  */
   if (handle->ep_intr >= 0)
     return 0;
 
-  /* Here comes products check for tokens which
+  /* Libusb actually detects the removal of USB device in use.
-     always have card inserted.  */
+     However, there is no good API to handle the removal (yet),
-  switch (handle->id_vendor)
+     cleanly and with good portability.
-    {
-    case VENDOR_FSIJ:
-      return 0;
-    }
 
+     There is libusb_set_pollfd_notifiers function, but it doesn't
+     offer libusb_device_handle* data to its callback.  So, when it
+     watches multiple devices, there is no way to know which device is
+     removed.
+
+     Once, we will have a good programming interface of libusb, we can
+     list tokens (with no interrupt transfer support, but always with
+     card inserted) here to return 0, so that scdaemon can submit
+     minimum packet on wire.
+  */
   return 1;
 }
 

scd/command.c

@@ -217,13 +217,18 @@ open_card_with_request (ctrl_t ctrl, const char *apptype, const char *serialno)
   gpg_error_t err;
   unsigned char *serialno_bin = NULL;
   size_t serialno_bin_len = 0;
+  app_t app = ctrl->app_ctx;
 
   /* If we are already initialized for one specific application we
      need to check that the client didn't requested a specific
      application different from the one in use before we continue. */
-  if (ctrl->app_ctx)
+  if (apptype && ctrl->app_ctx)
     return check_application_conflict (apptype, ctrl->app_ctx);
 
+  /* Re-scan USB devices.  Release APP, before the scan.  */
+  ctrl->app_ctx = NULL;
+  release_application (app);
+
   if (serialno)
     serialno_bin = hex_to_buffer (serialno, &serialno_bin_len);
 

scd/scdaemon.c

@@ -61,10 +61,10 @@
 
 enum cmd_and_opt_values
 { aNull = 0,
-  oCsh		  = 'c',
+  oCsh            = 'c',
-  oQuiet	  = 'q',
+  oQuiet          = 'q',
-  oSh		  = 's',
+  oSh             = 's',
-  oVerbose	  = 'v',
+  oVerbose        = 'v',
 
   oNoVerbose = 500,
   aGPGConfList,
@@ -115,11 +115,11 @@ static ARGPARSE_OPTS opts[] = {
                 N_("run in multi server mode (foreground)")),
   ARGPARSE_s_n (oDaemon, "daemon", N_("run in daemon mode (background)")),
   ARGPARSE_s_n (oVerbose, "verbose", N_("verbose")),
-  ARGPARSE_s_n (oQuiet,	"quiet", N_("be somewhat more quiet")),
+  ARGPARSE_s_n (oQuiet, "quiet", N_("be somewhat more quiet")),
-  ARGPARSE_s_n (oSh,	"sh", N_("sh-style command output")),
+  ARGPARSE_s_n (oSh,    "sh", N_("sh-style command output")),
-  ARGPARSE_s_n (oCsh,	"csh", N_("csh-style command output")),
+  ARGPARSE_s_n (oCsh,   "csh", N_("csh-style command output")),
   ARGPARSE_s_s (oOptions, "options", N_("|FILE|read options from FILE")),
-  ARGPARSE_s_s (oDebug,	"debug", "@"),
+  ARGPARSE_s_s (oDebug, "debug", "@"),
   ARGPARSE_s_n (oDebugAll, "debug-all", "@"),
   ARGPARSE_s_s (oDebugLevel, "debug-level" ,
                 N_("|LEVEL|set the debugging level to LEVEL")),
@@ -461,13 +461,13 @@ main (int argc, char **argv )
         parse_debug++;
       else if (pargs.r_opt == oOptions)
         { /* yes there is one, so we do not try the default one, but
-	     read the option file when it is encountered at the
+             read the option file when it is encountered at the
-	     commandline */
+             commandline */
           default_config = 0;
-	}
+        }
-	else if (pargs.r_opt == oNoOptions)
+        else if (pargs.r_opt == oNoOptions)
           default_config = 0; /* --no-options */
-	else if (pargs.r_opt == oHomedir)
+        else if (pargs.r_opt == oHomedir)
           gnupg_set_homedir (pargs.r.ret_str);
     }
 
@@ -502,16 +502,16 @@ main (int argc, char **argv )
               if( parse_debug )
                 log_info (_("Note: no default option file '%s'\n"),
                           configname );
-	    }
+            }
           else
             {
               log_error (_("option file '%s': %s\n"),
                          configname, strerror(errno) );
               exit(2);
-	    }
+            }
           xfree (configname);
           configname = NULL;
-	}
+        }
       if (parse_debug && configname )
         log_info (_("reading options from '%s'\n"), configname );
       default_config = 0;
@@ -558,10 +558,10 @@ main (int argc, char **argv )
           /* config files may not be nested (silently ignore them) */
           if (!configfp)
             {
-		xfree(configname);
+                xfree(configname);
-		configname = xstrdup(pargs.r.ret_str);
+                configname = xstrdup(pargs.r.ret_str);
-		goto next_pass;
+                goto next_pass;
-	    }
+            }
           break;
         case oNoGreeting: nogreeting = 1; break;
         case oNoVerbose: opt.verbose = 0; break;
@@ -593,12 +593,12 @@ main (int argc, char **argv )
           add_to_strlist (&opt.disabled_applications, pargs.r.ret_str);
           break;
 
-	case oEnablePinpadVarlen: opt.enable_pinpad_varlen = 1; break;
+        case oEnablePinpadVarlen: opt.enable_pinpad_varlen = 1; break;
 
         default:
           pargs.err = configfp? ARGPARSE_PRINT_WARNING:ARGPARSE_PRINT_ERROR;
           break;
-	}
+        }
     }
   if (configfp)
     {
@@ -661,7 +661,7 @@ main (int argc, char **argv )
       char *filename_esc;
 
       if (config_filename)
-	filename = xstrdup (config_filename);
+        filename = xstrdup (config_filename);
       else
         filename = make_filename (gnupg_homedir (),
                                   SCDAEMON_NAME EXTSEP_S "conf", NULL);
@@ -1035,7 +1035,7 @@ static void
 handle_tick (void)
 {
   if (!ticker_disabled)
-    scd_update_reader_status_file ();
+    usb_periodical_check = scd_update_reader_status_file ();
 }
 
 
@@ -1186,13 +1186,6 @@ start_connection_thread (void *arg)
 }
 
 
-void
-update_usb (int periodical_check_needed)
-{
-  usb_periodical_check = periodical_check_needed;
-  log_debug ("update_usb (%d)\n", periodical_check_needed);
-}
-
 void
 scd_kick_the_loop (void)
 {
@@ -1226,8 +1219,6 @@ handle_connections (int listen_fd)
   int nfd;
   int ret;
   int fd;
-  struct timespec abstime;
-  struct timespec curtime;
   struct timespec timeout;
   struct timespec *t;
   int saved_errno;
@@ -1275,12 +1266,6 @@ handle_connections (int listen_fd)
   if (nfd < pipe_fd[0])
     nfd = pipe_fd[0];
 
-  npth_clock_gettime (&curtime);
-  timeout.tv_sec = TIMERTICK_INTERVAL_SEC;
-  timeout.tv_nsec = TIMERTICK_INTERVAL_USEC * 1000;
-  npth_timeradd (&curtime, &timeout, &abstime);
-  /* We only require abstime here.  The others will be reused.  */
-
   for (;;)
     {
       if (shutdown_pending)
@@ -1298,25 +1283,16 @@ handle_connections (int listen_fd)
           listen_fd = -1;
         }
 
-      if ((listen_fd != -1 && nfd == listen_fd)
+      handle_tick ();
-          || need_tick ())
+
-        {
+      timeout.tv_sec = TIMERTICK_INTERVAL_SEC;
-          npth_clock_gettime (&curtime);
+      timeout.tv_nsec = TIMERTICK_INTERVAL_USEC * 1000;
-          if (!(npth_timercmp (&curtime, &abstime, <)))
+
-            {
+      if (need_tick ())
-              /* Timeout.  */
+        t = &timeout;
-              timeout.tv_sec = TIMERTICK_INTERVAL_SEC;
-              timeout.tv_nsec = TIMERTICK_INTERVAL_USEC * 1000;
-              npth_timeradd (&curtime, &timeout, &abstime);
-            }
-          npth_timersub (&abstime, &curtime, &timeout);
-          t = &timeout;
-        }
       else
         t = NULL;
 
-      handle_tick ();
-
       /* POSIX says that fd_set should be implemented as a structure,
          thus a simple assignment is fine to copy the entire set.  */
       read_fdset = fdset;

scd/scdaemon.h

@@ -123,9 +123,10 @@ int  scd_command_handler (ctrl_t, int);
 void send_status_info (ctrl_t ctrl, const char *keyword, ...)
      GPGRT_ATTR_SENTINEL(1);
 void send_status_direct (ctrl_t ctrl, const char *keyword, const char *args);
-void scd_update_reader_status_file (void);
 void send_client_notifications (app_t app, int removal);
-void update_usb (int periodical_check_needed);
 void scd_kick_the_loop (void);
 
+/*-- app.c --*/
+int scd_update_reader_status_file (void);
+
 #endif /*SCDAEMON_H*/