scd: Add explict functions for 'app' reference counting.

* scd/app.c (app_ref): New.
(app_unref): New.
(release_application): Renamed to ...
(app_unref_locked): this and remove arg locked_already.  Change
callers to use this or app_ref.
* scd/command.c (open_card_with_request):
(cmd_pksign, cmd_pkauth, cmd_pkdecrypt): Use app_ref and app_unref
instead of accessing the counter directly.
--

This is better in case we need to debug stuff.  There is a real change
however: We now lock and unlock the app before changing the reference
count.

The whole app locking business should be reviewed because we pass
pointers along without immediately bumping the refcount.

Signed-off-by: Werner Koch <wk@gnupg.org>

scd/app-common.h

@@ -164,7 +164,11 @@ gpg_error_t select_application (ctrl_t ctrl, const char *name, app_t *r_app,
                                 int scan, const unsigned char *serialno_bin,
                                 size_t serialno_bin_len);
 char *get_supported_applications (void);
-void release_application (app_t app, int locked_already);
+
+app_t app_ref (app_t app);
+void  app_unref (app_t app);
+void  app_unref_locked (app_t app);
+
 gpg_error_t app_munge_serialno (app_t app);
 gpg_error_t app_write_learn_status (app_t app, ctrl_t ctrl,
                                     unsigned int flags);

scd/app.c

@@ -242,7 +242,7 @@ app_reset (app_t app, ctrl_t ctrl, int send_reset)
   else
     {
       ctrl->app_ctx = NULL;
-      release_application (app, 0);
+      app_unref (app);
     }
 
   return err;
@@ -541,6 +541,7 @@ select_application (ctrl_t ctrl, const char *name, app_t *r_app,
       err = check_conflict (a, name);
       if (!err)
         {
+          /* Note: We do not use app_ref as we are already locked.  */
           a->ref_count++;
           *r_app = a;
           if (a_prev)
@@ -604,7 +605,8 @@ deallocate_app (app_t app)
       a_prev = a;
 
   if (app->ref_count)
-    log_error ("trying to release context used yet (%d)\n", app->ref_count);
+    log_error ("trying to release still used app context (%d)\n",
+               app->ref_count);
 
   if (app->fnc.deinit)
     {
@@ -618,13 +620,24 @@ deallocate_app (app_t app)
   xfree (app);
 }
 
-/* Free the resources associated with the application APP.  APP is
-   allowed to be NULL in which case this is a no-op.  Note that we are
-   using reference counting to track the users of the application and
-   actually deferring the deallocation to allow for a later reuse by
-   a new connection. */
+
+/* Increment the reference counter for APP.  Returns the APP.  */
+app_t
+app_ref (app_t app)
+{
+  lock_app (app, NULL);
+  ++app->ref_count;
+  unlock_app (app);
+  return app;
+}
+
+
+/* Decrement the reference counter for APP.  Note that we are using
+ * reference counting to track the users of the application and are
+ * deferring the actual deallocation to allow for a later reuse by a
+ * new connection.  Using NULL for APP is a no-op. */
 void
-release_application (app_t app, int locked_already)
+app_unref (app_t app)
 {
   if (!app)
     return;
@@ -634,15 +647,26 @@ release_application (app_t app, int locked_already)
      is using the card - this way the PIN cache and other cached data
      are preserved.  */
 
-  if (!locked_already)
-    lock_app (app, NULL);
+  lock_app (app, NULL);
+  if (!app->ref_count)
+    log_bug ("trying to release an already released context\n");
+  --app->ref_count;
+  unlock_app (app);
+}
+
+
+/* This is the same as app_unref but assumes that APP is already
+ * locked.  */
+void
+app_unref_locked (app_t app)
+{
+  if (!app)
+    return;
 
   if (!app->ref_count)
     log_bug ("trying to release an already released context\n");
 
   --app->ref_count;
-  if (!locked_already)
-    unlock_app (app);
 }
 
 

scd/command.c

@@ -234,7 +234,7 @@ open_card_with_request (ctrl_t ctrl, const char *apptype, const char *serialno)
 
   /* Re-scan USB devices.  Release APP, before the scan.  */
   ctrl->app_ctx = NULL;
-  release_application (app, 0);
+  app_unref (app);
 
   if (serialno)
     serialno_bin = hex_to_buffer (serialno, &serialno_bin_len);
@@ -804,14 +804,14 @@ cmd_pksign (assuan_context_t ctx, char *line)
   if (app)
     {
       if (direct)
-        app->ref_count++;
+        app_ref (app);
       rc = app_sign (app, ctrl,
                      keyidstr, hash_algo,
                      pin_cb, ctx,
                      ctrl->in_data.value, ctrl->in_data.valuelen,
                      &outdata, &outdatalen);
       if (direct)
-        app->ref_count--;
+        app_unref (app);
     }
   else
     rc = gpg_error (GPG_ERR_NO_SECKEY);
@@ -872,12 +872,12 @@ cmd_pkauth (assuan_context_t ctx, char *line)
   if (app)
     {
       if (direct)
-        app->ref_count++;
+        app_ref (app);
       rc = app_auth (app, ctrl, keyidstr, pin_cb, ctx,
                      ctrl->in_data.value, ctrl->in_data.valuelen,
                      &outdata, &outdatalen);
       if (direct)
-        app->ref_count--;
+        app_unref (app);
     }
   else
     rc = gpg_error (GPG_ERR_NO_SECKEY);
@@ -933,12 +933,12 @@ cmd_pkdecrypt (assuan_context_t ctx, char *line)
   if (app)
     {
       if (direct)
-        app->ref_count++;
+        app_ref (app);
       rc = app_decipher (ctrl->app_ctx, ctrl, keyidstr, pin_cb, ctx,
                          ctrl->in_data.value, ctrl->in_data.valuelen,
                          &outdata, &outdatalen, &infoflags);
       if (direct)
-        app->ref_count--;
+        app_unref (app);
     }
   else
     rc = gpg_error (GPG_ERR_NO_SECKEY);
@@ -1648,7 +1648,7 @@ cmd_restart (assuan_context_t ctx, char *line)
   if (app)
     {
       ctrl->app_ctx = NULL;
-      release_application (app, 0);
+      app_unref (app);
     }
   if (locked_session && ctrl->server_local == locked_session)
     {
@@ -2169,7 +2169,8 @@ popup_prompt (void *opaque, int on)
 }
 
 
-/* Helper to send the clients a status change notification.  */
+/* Helper to send the clients a status change notification.  Note that
+ * this fucntion assumes that APP is already locked.  */
 void
 send_client_notifications (app_t app, int removal)
 {
@@ -2199,7 +2200,7 @@ send_client_notifications (app_t app, int removal)
           {
             sl->ctrl_backlink->app_ctx = NULL;
             sl->card_removed = 1;
-            release_application (app, 1);
+            app_unref_locked (app);
           }
 
         if (!sl->event_signal || !sl->assuan_ctx)

scd/scdaemon.h

@@ -129,7 +129,10 @@ void send_keyinfo (ctrl_t ctrl, int data, const char *keygrip_str,
                    const char *serialno, const char *idstr);
 
 void popup_prompt (void *opaque, int on);
+
+/* Take care: this function assumes that APP is locked.  */
 void send_client_notifications (app_t app, int removal);
+
 void scd_kick_the_loop (void);
 int get_active_connection_count (void);