Unclear parts of ssh_options_set() and ssh_options_get()

[Jakub Jelen <jjelen@xxxxxxxxxx>]

Hello,
while checking the options handling and its documentation I found few
unclear/confusing points:

 * The *get() function lists SSH_OPTIONS_ADD_IDENTITY, but the
description more fits for the set() function, which is missing this
option in documentation, but implements it.

 * The description of the SSH_OPTIONS_IDENTITY in the get() function is
the same as the description of the set(), which sounds confusing. It is
probably copy&paste error.

 * SSH_OPTIONS_IDENTITY does the same thing as SSH_OPTIONS_ADD_IDENTITY
according to the source code, but has different description (both of
them are prepending the identity to the list).

 * The get() function has SSH_OPTIONS_IDENTITY option, but from the
source code it looks like it is returning just the first identity from
the list. Is there some simple way to get all of them or to work with
them?

 * Several options talk about substitution sequences as %s, but if the
ssh_path_expand_escape() function is used, there is only %d sequence,
which is closest to the described behavior. This survived from some old
version of libssh, where the replacement was done directly using
snprintf() function (f6d2a66). Now, there are more expansion sequences,
but it is probably not worth mentioning all of them here.

 * It might be little bit confusing that the substitution sequences are
little bit different than in OpenSSH (%d points to user home directory
in OpenSSH). But we are probably late to change this one since it is
already used all over the code.

I am attaching an initial version of the patch, but feel free to
improve the wording or descriptions if I got something wrongly.

Regards,
-- 
Jakub Jelen
Software Engineer
Security Technologies
Red Hat, Inc.

From 905023024f1081138d6f35ad5855e08fc7e6c9b8 Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jjelen@xxxxxxxxxx>
Date: Mon, 13 Nov 2017 14:48:23 +0100
Subject: [PATCH 1/3] options: Move SSH_OPTIONS_ADD_IDENTITY to *set() function
 description

Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx>
---
 src/options.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/src/options.c b/src/options.c
index 9346b6d1..75fd6bb2 100644
--- a/src/options.c
+++ b/src/options.c
@@ -238,12 +238,14 @@ int ssh_options_set_algo(ssh_session session,
  *                are genuine. It may include "%s" which will be
  *                replaced by the user home directory.
  *
- *              - SSH_OPTIONS_IDENTITY:
- *                Set the identity file name (const char *,format string).\n
+ *              - SSH_OPTIONS_IDENTITY, SSH_OPTIONS_ADD_IDENTITY:
+ *                Add a new identity file (const char *,format string) to
+ *                the identity list.\n
  *                \n
  *                By default identity, id_dsa and id_rsa are checked.\n
  *                \n
- *                The identity file used authenticate with public key.
+ *                The identity used authenticate with public key will be
+ *                prepended to the list.
  *                It may include "%s" which will be replaced by the
  *                user home directory.
  *
@@ -926,17 +928,6 @@ int ssh_options_get_port(ssh_session session, unsigned int* port_target) {
  *                It may include "%s" which will be replaced by the
  *                user home directory.
  *
- *              - SSH_OPTIONS_ADD_IDENTITY:
- *                Add a new identity file (const char *,format string) to
- *                the identity list.\n
- *                \n
- *                By default identity, id_dsa and id_rsa are checked.\n
- *                \n
- *                The identity used authenticate with public key will be
- *                prepended to the list.
- *                It may include "%s" which will be replaced by the
- *                user home directory.
- *
  *              - SSH_OPTIONS_PROXYCOMMAND:
  *                Get the proxycommand necessary to log into the
  *                remote host. When not explicitly set, it will be read
-- 
2.13.6


From 0378af41532a1bdff741a22bccb106c96e6a0f7a Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jjelen@xxxxxxxxxx>
Date: Mon, 13 Nov 2017 14:52:23 +0100
Subject: [PATCH 2/3] options: Rewrite set() description to get()

Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx>
---
 src/options.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/options.c b/src/options.c
index 75fd6bb2..5f7412d3 100644
--- a/src/options.c
+++ b/src/options.c
@@ -920,13 +920,9 @@ int ssh_options_get_port(ssh_session session, unsigned int* port_target) {
  *                ~/.ssh/config file.
  *
  *              - SSH_OPTIONS_IDENTITY:
- *                Set the identity file name (const char *,format string).\n
+ *                Get the first identity file name (const char *).\n
  *                \n
- *                By default identity, id_dsa and id_rsa are checked.\n
- *                \n
- *                The identity file used authenticate with public key.
- *                It may include "%s" which will be replaced by the
- *                user home directory.
+ *                By default identity, id_dsa and id_rsa are checked.
  *
  *              - SSH_OPTIONS_PROXYCOMMAND:
  *                Get the proxycommand necessary to log into the
-- 
2.13.6


From e2a146b9b0721e82728cfe220d0a2daf5c5dc69c Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jjelen@xxxxxxxxxx>
Date: Mon, 13 Nov 2017 15:10:40 +0100
Subject: [PATCH 3/3] options: get rid of %s replace sequences

Signed-off-by: Jakub Jelen <jjelen@xxxxxxxxxx>
---
 src/options.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/options.c b/src/options.c
index 5f7412d3..9bf59beb 100644
--- a/src/options.c
+++ b/src/options.c
@@ -224,7 +224,7 @@ int ssh_options_set_algo(ssh_session session,
  *                \n
  *                The ssh directory is used for files like known_hosts
  *                and identity (private and public key). It may include
- *                "%s" which will be replaced by the user home
+ *                "~" which will be replaced by the user home
  *                directory.
  *
  *              - SSH_OPTIONS_KNOWNHOSTS:
@@ -235,8 +235,8 @@ int ssh_options_set_algo(ssh_session session,
  *                ~/.ssh/known_hosts.\n
  *                \n
  *                The known hosts file is used to certify remote hosts
- *                are genuine. It may include "%s" which will be
- *                replaced by the user home directory.
+ *                are genuine. It may include "%d" which will be
+ *                replaced by the ssh directory (SSH_OPTIONS_SSH_DIR).
  *
  *              - SSH_OPTIONS_IDENTITY, SSH_OPTIONS_ADD_IDENTITY:
  *                Add a new identity file (const char *,format string) to
@@ -246,8 +246,8 @@ int ssh_options_set_algo(ssh_session session,
  *                \n
  *                The identity used authenticate with public key will be
  *                prepended to the list.
- *                It may include "%s" which will be replaced by the
- *                user home directory.
+ *                It may include "%d" which will be replaced by the
+ *                ssh directory (set with SSH_OPTIONS_SSH_DIR).
  *
  *              - SSH_OPTIONS_TIMEOUT:
  *                Set a timeout for the connection in seconds (long).
-- 
2.13.6