RE: [PATCH v2] Remove sshkey_load_private()

["Sharma, Jitendra" <jitendra.sharma@xxxxxxxxx>]

Thanks Damien for replying.

> -----Original Message-----
> From: Damien Miller [mailto:djm@xxxxxxxxxxx]
> Sent: Thursday, August 8, 2019 4:57 AM
> To: Sharma, Jitendra <jitendra.sharma@xxxxxxxxx>
> Cc: openssh-unix-dev@xxxxxxxxxxx
> Subject: Re: [PATCH v2] Remove sshkey_load_private()
> 
> I think I'd prefer that sshkey_load_private() remains as a wrapper.
> 

I assume there must be a strong reason for keeping sshkey_load_private wrapper, which I am not aware of ?
However if the intent is to provide a facility/wrapper to consumers of this function, then to avoid duplicate code, we could provide a macro interface to end users. 
Could you please review this code, if it sounds good:

jitendra@clear-linux~/openssh-portable $ git diff
diff --git a/authfile.c b/authfile.c
index 5e335ce4..89b5ca39 100644
--- a/authfile.c
+++ b/authfile.c
@@ -215,44 +215,6 @@ sshkey_load_private_type_fd(int fd, int type, const char *passphrase,
        return r;
 }

-/* XXX this is almost identical to sshkey_load_private_type() */
-int
-sshkey_load_private(const char *filename, const char *passphrase,
-    struct sshkey **keyp, char **commentp)
-{
-       struct sshbuf *buffer = NULL;
-       int r, fd;
-
-       if (keyp != NULL)
-               *keyp = NULL;
-       if (commentp != NULL)
-               *commentp = NULL;
-
-       if ((fd = open(filename, O_RDONLY)) == -1)
-               return SSH_ERR_SYSTEM_ERROR;
-       if (sshkey_perm_ok(fd, filename) != 0) {
-               r = SSH_ERR_KEY_BAD_PERMISSIONS;
-               goto out;
-       }
-
-       if ((buffer = sshbuf_new()) == NULL) {
-               r = SSH_ERR_ALLOC_FAIL;
-               goto out;
-       }
-       if ((r = sshkey_load_file(fd, buffer)) != 0 ||
-           (r = sshkey_parse_private_fileblob(buffer, passphrase, keyp,
-           commentp)) != 0)
-               goto out;
-       if (keyp && *keyp &&
-           (r = sshkey_set_filename(*keyp, filename)) != 0)
-               goto out;
-       r = 0;
- out:
-       close(fd);
-       sshbuf_free(buffer);
-       return r;
-}
-
 static int
 sshkey_try_load_public(struct sshkey *k, const char *filename, char **commentp)
 {
diff --git a/authfile.h b/authfile.h
index 54df169b..2962fbba 100644
--- a/authfile.h
+++ b/authfile.h
@@ -30,6 +30,9 @@
 struct sshbuf;
 struct sshkey;

+#define sshkey_load_private(filename, passphrase, keyp, commentp) \
+       sshkey_load_private_type(KEY_UNSPEC, filename, passphrase, keyp, commentp)
+
 /* XXX document these */
 /* XXX some of these could probably be merged/retired */

@@ -38,7 +41,6 @@ int sshkey_save_private(struct sshkey *, const char *,
 int sshkey_load_file(int, struct sshbuf *);
 int sshkey_load_cert(const char *, struct sshkey **);
 int sshkey_load_public(const char *, struct sshkey **, char **);
-int sshkey_load_private(const char *, const char *, struct sshkey **, char **);
 int sshkey_load_private_cert(int, const char *, const char *,
     struct sshkey **);
 int sshkey_load_private_type(int, const char *, const char *,
(END)


> On Tue, 6 Aug 2019, Jitendra Sharma wrote:
> 
> > Remove sshkey_load_private(), as this function's role is similar to
> > sshkey_load_private_type().
> > ---
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev