Re: [PATCH 1/3] Separate out key import functionality from ssh_bind_listen

[Andreas Schneider <asn@xxxxxxxxxxxxxx>]

On Friday 17 January 2014 17:01:49 Alan Dunn wrote:
> Signed-off-by: Alan Dunn <amdunn@xxxxxxxxx>

Hi Alan,


thank you very much for your patchset! I have just some minor things I would 
like to have changed. It is for readabilty and easier debugging :)

> ---
>  src/bind.c |   33 +++++++++++++++++++++------------
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/src/bind.c b/src/bind.c
> index 698b953..e391738 100644
> --- a/src/bind.c
> +++ b/src/bind.c
> @@ -144,26 +144,19 @@ ssh_bind ssh_bind_new(void) {
>    return ptr;
>  }
> 
> -int ssh_bind_listen(ssh_bind sshbind) {
> -  const char *host;
> -  socket_t fd;
> +static int ssh_bind_import_keys(ssh_bind sshbind) {
>    int rc;
> 
> -  if (ssh_init() < 0) {
> -    ssh_set_error(sshbind, SSH_FATAL, "ssh_init() failed");
> -    return -1;
> -  }
> -
>    if (sshbind->ecdsakey == NULL &&
>        sshbind->dsakey == NULL &&
>        sshbind->rsakey == NULL) {
>        ssh_set_error(sshbind, SSH_FATAL,
> -              "DSA or RSA host key file must be set before listen()");
> +                    "ECDSA, DSA, or RSA host key file must be set");
>        return SSH_ERROR;
>    }
> 
>  #ifdef HAVE_ECC
> -  if (sshbind->ecdsakey) {
> +  if (sshbind->ecdsakey && !sshbind->ecdsa) {

Please make them more verbose for better readability:

if (sshbind->ecdsa == NULL && sshbind->ecdsakey != NULL) {

>        rc = ssh_pki_import_privkey_file(sshbind->ecdsakey,
>                                         NULL,
>                                         NULL,
> @@ -185,7 +178,7 @@ int ssh_bind_listen(ssh_bind sshbind) {
>    }
>  #endif
> 
> -  if (sshbind->dsakey) {
> +  if (sshbind->dsakey && !sshbind->dsa) {
>        rc = ssh_pki_import_privkey_file(sshbind->dsakey,
>                                         NULL,
>                                         NULL,
> @@ -207,7 +200,7 @@ int ssh_bind_listen(ssh_bind sshbind) {
>        }
>    }
> 
> -  if (sshbind->rsakey) {
> +  if (sshbind->rsakey && !sshbind->rsa) {
>        rc = ssh_pki_import_privkey_file(sshbind->rsakey,
>                                         NULL,
>                                         NULL,
> @@ -229,6 +222,22 @@ int ssh_bind_listen(ssh_bind sshbind) {
>        }
>    }
> 
> +  return SSH_OK;
> +}
> +
> +int ssh_bind_listen(ssh_bind sshbind) {
> +  const char *host;
> +  socket_t fd;

int rc;

> +
> +  if (ssh_init() < 0) {

rc = ssh_init();
if (rc < 0)

> +    ssh_set_error(sshbind, SSH_FATAL, "ssh_init() failed");
> +    return -1;
> +  }
> +
> +  if (ssh_bind_import_keys(sshbind) != SSH_OK) {
> +    return SSH_ERROR;
> +  }

rc = ssh_bind_import_keys(sshbind);
...

> +
>    if (sshbind->bindfd == SSH_INVALID_SOCKET) {
>        host = sshbind->bindaddr;
>        if (host == NULL) {

-- 
Andreas Schneider                   GPG-ID: CC014E3D
www.cryptomilk.org                asn@xxxxxxxxxxxxxx