Amend the agent code with more comments.

* agent/command.c (server_local_s): Remove unused field MESSAGE_FD.

3 files changed, 161 insertions, 46 deletions

diff --git a/agent/agent.h b/agent/agent.h
index b323718fc..4f4e477a6 100644
--- a/agent/agent.h
+++ b/agent/agent.h
@@ -73,17 +73,20 @@ struct
   /* True if we handle sigusr2.  */
   int sigusr2_enabled;
 
-  /* Environment setting gathered at program start or changed using the
+  /* Environment settings gathered at program start or changed using the
      Assuan command UPDATESTARTUPTTY. */
   session_env_t startup_env;
   char *startup_lc_ctype;
   char *startup_lc_messages;
 
-  const char *pinentry_program; /* Filename of the program to start as
-                                   pinentry.  */
-  const char *scdaemon_program; /* Filename of the program to handle
-                                   smartcard tasks.  */
+  /* Filename of the program to start as pinentry.  */
+  const char *pinentry_program;
+
+  /* Filename of the program to handle smartcard tasks.  */
+  const char *scdaemon_program;
+
   int disable_scdaemon;         /* Never use the SCdaemon. */
+
   int no_grab;         /* Don't let the pinentry grab the keyboard */
 
   /* The name of the file pinentry shall tocuh before exiting.  If
@@ -98,31 +101,51 @@ struct
 
   /* Flag disallowing bypassing of the warning.  */
   int enforce_passphrase_constraints;
+
   /* The require minmum length of a passphrase. */
   unsigned int min_passphrase_len;
+
   /* The minimum number of non-alpha characters in a passphrase.  */
   unsigned int min_passphrase_nonalpha;
+
   /* File name with a patternfile or NULL if not enabled.  */
   const char *check_passphrase_pattern;
+
   /* If not 0 the user is asked to change his passphrase after these
      number of days.  */
   unsigned int max_passphrase_days;
+
   /* If set, a passphrase history will be written and checked at each
      passphrase change.  */
   int enable_passhrase_history;
 
   int running_detached; /* We are running detached from the tty. */
 
+  /* If this global option is true, the passphrase cache is ignored
+     for signing operations.  */
   int ignore_cache_for_signing;
+
+  /* If this global option is true, the user is allowed to
+     interactively mark certificate in trustlist.txt as trusted. */
   int allow_mark_trusted;
+
+  /* If this global option is true, the Assuan command
+     PRESET_PASSPHRASE is allowed.  */
   int allow_preset_passphrase;
+
+  /* If this global option is true, the Assuan option
+     pinentry-mode=loopback is allowed.  */
   int allow_loopback_pinentry;
+
   int keep_tty;      /* Don't switch the TTY (for pinentry) on request */
   int keep_display;  /* Don't switch the DISPLAY (for pinentry) on request */
-  int ssh_support;   /* Enable ssh-agent emulation.  */
+
+  /* This global option enables the ssh-agent subsystem.  */
+  int ssh_support;
 } opt;
 
 
+/* Bit values for the --debug option.  */
 #define DBG_COMMAND_VALUE 1	/* debug commands i/o */
 #define DBG_MPI_VALUE	  2	/* debug mpi details */
 #define DBG_CRYPTO_VALUE  4	/* debug low level crypto */
@@ -130,8 +153,9 @@ struct
 #define DBG_CACHE_VALUE   64	/* debug the caching */
 #define DBG_MEMSTAT_VALUE 128	/* show memory statistics */
 #define DBG_HASHING_VALUE 512	/* debug hashing operations */
-#define DBG_ASSUAN_VALUE 1024
+#define DBG_ASSUAN_VALUE 1024   /* Enable Assuan debugging.  */
 
+/* Test macros for the debug option.  */
 #define DBG_COMMAND (opt.debug & DBG_COMMAND_VALUE)
 #define DBG_CRYPTO  (opt.debug & DBG_CRYPTO_VALUE)
 #define DBG_MEMORY  (opt.debug & DBG_MEMORY_VALUE)
@@ -139,14 +163,18 @@ struct
 #define DBG_HASHING (opt.debug & DBG_HASHING_VALUE)
 #define DBG_ASSUAN  (opt.debug & DBG_ASSUAN_VALUE)
 
+/* Forward reference for local definitions in command.c.  */
 struct server_local_s;
+
+/* Forward reference for local definitions in call-scd.c.  */
 struct scd_local_s;
 
 /* Collection of data per session (aka connection). */
 struct server_control_s
 {
   /* Private data used to fire up the connection thread.  We use this
-     structure do avoid an extra allocation for just a few bytes. */
+     structure do avoid an extra allocation for only a few bytes while
+     spawning a new connection thread.  */
   struct {
     gnupg_fd_t fd;
   } thread_startup;
@@ -157,6 +185,7 @@ struct server_control_s
   /* Private data of the SCdaemon (call-scd.c). */
   struct scd_local_s *scd_local;
 
+  /* Environment settings for the connection.  */
   session_env_t session_env;
   char *lc_ctype;
   char *lc_messages;
@@ -177,37 +206,47 @@ struct server_control_s
   unsigned char keygrip[20];
   int have_keygrip;
 
-  int use_auth_call; /* Hack to send the PKAUTH command instead of the
-                        PKSIGN command to the scdaemon.  */
-  int in_passwd;     /* Hack to inhibit enforced passphrase change
-                        during an explicit passwd command.  */
+  /* A flag to enable a hack to send the PKAUTH command instead of the
+     PKSIGN command to the scdaemon.  */
+  int use_auth_call;
+
+  /* A flag to inhibit enforced passphrase change during an explicit
+     passwd command.  */
+  int in_passwd;
 
-  unsigned long s2k_count; /* Other than the calibrated count. */
+  /* The current S2K which might be different from the calibrated
+     count. */
+  unsigned long s2k_count;
 };
 
 
+/* Information pertaining to pinentry requests.  */
 struct pin_entry_info_s
 {
   int min_digits; /* min. number of digits required or 0 for freeform entry */
   int max_digits; /* max. number of allowed digits allowed*/
-  int max_tries;
-  int failed_tries;
+  int max_tries;  /* max. number of allowed tries.  */
+  int failed_tries; /* Number of tries so far failed.  */
   int with_qualitybar; /* Set if the quality bar should be displayed.  */
   int (*check_cb)(struct pin_entry_info_s *); /* CB used to check the PIN */
   void *check_cb_arg;  /* optional argument which might be of use in the CB */
   const char *cb_errtext; /* used by the cb to display a specific error */
-  size_t max_length;      /* allocated length of the buffer */
-  char pin[1];
+  size_t max_length;   /* Allocated length of the buffer PIN. */
+  char pin[1];         /* The buffer to hold the PIN or passphrase.
+                          It's actual allocated length is given by
+                          MAX_LENGTH (above).  */
 };
 
 
+/* Types of the private keys.  */
 enum
   {
-    PRIVATE_KEY_UNKNOWN = 0,
-    PRIVATE_KEY_CLEAR = 1,
-    PRIVATE_KEY_PROTECTED = 2,
-    PRIVATE_KEY_SHADOWED = 3,
-    PROTECTED_SHARED_SECRET = 4
+    PRIVATE_KEY_UNKNOWN = 0,      /* Type of key is not known.  */
+    PRIVATE_KEY_CLEAR = 1,        /* The key is not protected.  */
+    PRIVATE_KEY_PROTECTED = 2,    /* The key is protected.  */
+    PRIVATE_KEY_SHADOWED = 3,     /* The key is a stub for a smartcard
+                                     based key.  */
+    PROTECTED_SHARED_SECRET = 4   /* RFU.  */
   };
 
 
diff --git a/agent/command.c b/agent/command.c
index f310a980c..0e7b962c4 100644
--- a/agent/command.c
+++ b/agent/command.c
@@ -50,31 +50,57 @@
 /* The size of the import/export KEK key (in bytes).  */
 #define KEYWRAP_KEYSIZE (128/8)
 
+/* A shortcut to call assuan_set_error using an gpg_err_code_t and a
+   text string.  */
 #define set_error(e,t) assuan_set_error (ctx, gpg_error (e), (t))
 
-
+/* Check that the maximum digest length we support has at least the
+   length of the keygrip.  */
 #if MAX_DIGEST_LEN < 20
 #error MAX_DIGEST_LEN shorter than keygrip
 #endif
 
-/* Data used to associate an Assuan context with local server data */
+/* Data used to associate an Assuan context with local server data.
+   This is this modules local part of the server_control_s struct.  */
 struct server_local_s
 {
+  /* Our Assuan context.  */
   assuan_context_t assuan_ctx;
-  int message_fd;
+
+  /* If this flag is true, the passphrase cache is used for signing
+     operations.  It defaults to true but may be set on a per
+     connection base.  The global option opt.ignore_cache_for_signing
+     takes precedence over this flag.  */
   int use_cache_for_signing;
-  char *keydesc;  /* Allocated description for the next key
-                     operation. */
-  int pause_io_logging; /* Used to suppress I/O logging during a command */
-  int stopme;    /* If set to true the agent will be terminated after
-                    the end of this session.  */
-  int allow_pinentry_notify; /* Set if pinentry notifications should
-                                be done. */
-  void *import_key;  /* Malloced KEK for the import_key command.  */
-  void *export_key;  /* Malloced KEK for the export_key command.  */
-  int allow_fully_canceled; /* Client is aware of GPG_ERR_FULLY_CANCELED.  */
-  char *last_cache_nonce;   /* Last CACHE_NOCNE sent as status (malloced).  */
-  char *last_passwd_nonce;  /* Last PASSWD_NOCNE sent as status (malloced). */
+
+  /* An allocated description for the next key operation.  This is
+     used if a pinnetry needs to be popped up.  */
+  char *keydesc;
+
+  /* Flags to suppress I/O logging during a command.  */
+  int pause_io_logging;
+
+  /* If this flags is set to true the agent will be terminated after
+     the end of the current session.  */
+  int stopme;
+
+  /* Flag indicating whether pinentry notifications shall be done. */
+  int allow_pinentry_notify;
+
+  /* Malloced KEK (Key-Encryption-Key) for the import_key command.  */
+  void *import_key;
+
+  /* Malloced KEK for the export_key command.  */
+  void *export_key;
+
+  /* Client is aware of the error code GPG_ERR_FULLY_CANCELED.  */
+  int allow_fully_canceled;
+
+  /* Last CACHE_NONCE sent as status (malloced).  */
+  char *last_cache_nonce;
+
+  /* Last PASSWD_NONCE sent as status (malloced). */
+  char *last_passwd_nonce;
 };
 
 
@@ -156,6 +182,8 @@ write_and_clear_outbuf (assuan_context_t ctx, membuf_t *mb)
 }
 
 
+/* Clear the nonces used to enable the passphrase cache for certain
+   multi-command command sequences.  */
 static void
 clear_nonce_cache (ctrl_t ctrl)
 {
@@ -176,6 +204,9 @@ clear_nonce_cache (ctrl_t ctrl)
 }
 
 
+/* This function is called by Libassuan whenever thee client sends a
+   reset.  It has been registered similar to the other Assuan
+   commands.  */
 static gpg_error_t
 reset_notify (assuan_context_t ctx, char *line)
 {
@@ -196,8 +227,13 @@ reset_notify (assuan_context_t ctx, char *line)
 }
 
 
-/* Skip over options.
-   Blanks after the options are also removed. */
+/* Skip over options in LINE.
+
+   Blanks after the options are also removed.  Options are indicated
+   by two leading dashes followed by a string consisting of non-space
+   characters.  The special option "--" indicates an explicit end of
+   options; all what follows will not be considered an option.  The
+   first no-option string also indicates the end of option parsing. */
 static char *
 skip_options (const char *line)
 {
@@ -213,7 +249,11 @@ skip_options (const char *line)
   return (char*)line;
 }
 
-/* Check whether the option NAME appears in LINE */
+
+/* Check whether the option NAME appears in LINE.  An example for a
+   line with options is:
+     --algo=42 --data foo bar
+   This function would then only return true if NAME is "data".  */
 static int
 has_option (const char *line, const char *name)
 {
@@ -226,6 +266,7 @@ has_option (const char *line, const char *name)
   return (s && (s == line || spacep (s-1)) && (!s[n] || spacep (s+n)));
 }
 
+
 /* Same as has_option but does only test for the name of the option
    and ignores an argument, i.e. with NAME being "--hash" it would
    return true for "--hash" as well as for "--hash=foo". */
@@ -242,8 +283,9 @@ has_option_name (const char *line, const char *name)
           && (!s[n] || spacep (s+n) || s[n] == '='));
 }
 
+
 /* Return a pointer to the argument of the option with NAME.  If such
-   an option is not given, it returns NULL. */
+   an option is not given, NULL is retruned. */
 static char *
 option_value (const char *line, const char *name)
 {
@@ -265,7 +307,7 @@ option_value (const char *line, const char *name)
 }
 
 
-/* Replace all '+' by a blank. */
+/* Replace all '+' by a blank in the string S. */
 static void
 plus_to_blank (char *s)
 {
@@ -296,8 +338,9 @@ parse_hexstring (assuan_context_t ctx, const char *string, size_t *len)
   return 0;
 }
 
+
 /* Parse the keygrip in STRING into the provided buffer BUF.  BUF must
-   provide space for 20 bytes. BUF is not changed if the function
+   provide space for 20 bytes.  BUF is not changed if the function
    returns an error. */
 static int
 parse_keygrip (assuan_context_t ctx, const char *string, unsigned char *buf)
@@ -319,7 +362,11 @@ parse_keygrip (assuan_context_t ctx, const char *string, unsigned char *buf)
 }
 
 
-/* Write an assuan status line. */
+/* Write an Assuan status line.  KEYWORD is the first item on the
+   status line.  The following arguments are all separated by a space
+   in the output.  The last argument must be a NULL.  Linefeeds and
+   carriage returns characters (which are not allowed in an Assuan
+   status line) are silently quoted in C-style.  */
 gpg_error_t
 agent_write_status (ctrl_t ctrl, const char *keyword, ...)
 {
@@ -463,6 +510,7 @@ bump_key_eventcounter (void)
   eventcounter.any++;
 }
 
+
 /* This function should be called for all card reader status
    changes.  This function is assured not to do any context
    switches. */
@@ -1069,6 +1117,8 @@ do_one_keyinfo (ctrl_t ctrl, const unsigned char *grip, assuan_context_t ctx,
 }
 
 
+/* Entry int for the command KEYINFO.  This function handles the
+   command option processing.  For details see hlp_keyinfo above.  */
 static gpg_error_t
 cmd_keyinfo (assuan_context_t ctx, char *line)
 {
@@ -1140,6 +1190,7 @@ cmd_keyinfo (assuan_context_t ctx, char *line)
 
 
 
+/* Helper for cmd_get_passphrase.  */
 static int
 send_back_passphrase (assuan_context_t ctx, int via_data, const char *pw)
 {
@@ -2415,6 +2466,8 @@ cmd_getinfo (assuan_context_t ctx, char *line)
 
 
 
+/* This function is called by Libassuan to parse the OPTION command.
+   It has been registered similar to the other Assuan commands.  */
 static gpg_error_t
 option_handler (assuan_context_t ctx, const char *key, const char *value)
 {
@@ -2574,7 +2627,8 @@ command_has_option (const char *cmd, const char *cmdopt)
 }
 
 
-/* Tell the assuan library about our commands */
+/* Tell Libassuan about our commands.  Also register the other Assuan
+   handlers. */
 static int
 register_commands (assuan_context_t ctx)
 {
@@ -2733,6 +2787,8 @@ start_command_handler (ctrl_t ctrl, gnupg_fd_t listen_fd, gnupg_fd_t fd)
 }
 
 
+/* Helper for the pinentry loopback mode.  It merely passes the
+   parameters on to the client.  */
 gpg_error_t
 pinentry_loopback(ctrl_t ctrl, const char *keyword,
                   unsigned char **buffer, size_t *size,
diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c
index 061687550..6dcde26af 100644
--- a/agent/gpg-agent.c
+++ b/agent/gpg-agent.c
@@ -309,6 +309,9 @@ static unsigned long pth_thread_id (void)
    Functions.
  */
 
+/* Allocate a string describing a library version by calling a GETFNC.
+   This function is expected to be called only once.  GETFNC is
+   expected to have a semantic like gcry_check_version ().  */
 static char *
 make_libversion (const char *libname, const char *(*getfnc)(const char*))
 {
@@ -326,7 +329,9 @@ make_libversion (const char *libname, const char *(*getfnc)(const char*))
   return result;
 }
 
-
+/* Return strings describing this program.  The case values are
+   described in common/argparse.c:strusage.  The values here override
+   the default values given by strusage.  */
 static const char *
 my_strusage (int level)
 {
@@ -448,6 +453,9 @@ remove_socket (char *name)
     }
 }
 
+
+/* Cleanup code for this program.  This is either called has an atexit
+   handler or directly.  */
 static void
 cleanup (void)
 {
@@ -1268,6 +1276,8 @@ main (int argc, char **argv )
 }
 
 
+/* Exit entry point.  This function should be called instead of a
+   plain exit.  */
 void
 agent_exit (int rc)
 {
@@ -1294,6 +1304,11 @@ agent_exit (int rc)
 }
 
 
+/* Each thread has its own local variables conveyed by a control
+   structure usually identified by an argument named CTRL.  This
+   function is called immediately after allocating the control
+   structure.  Its purpose is to setup the default values for that
+   structure.  */
 static void
 agent_init_default_ctrl (ctrl_t ctrl)
 {
@@ -1319,6 +1334,8 @@ agent_init_default_ctrl (ctrl_t ctrl)
 }
 
 
+/* Release all resources allocated by default in the control
+   structure.  This is the counterpart to agent_init_default_ctrl.  */
 static void
 agent_deinit_default_ctrl (ctrl_t ctrl)
 {
@@ -1720,6 +1737,7 @@ agent_sighup_action (void)
 }
 
 
+/* A helper function to handle SIGUSR2.  */
 static void
 agent_sigusr2_action (void)
 {
@@ -1730,6 +1748,8 @@ agent_sigusr2_action (void)
 }
 
 
+/* The signal handler for this program.  It is expected to be run in
+   its own trhead and not in the context of a signal handler.  */
 static void
 handle_signal (int signo)
 {