One rule we have is that when an allocation fails, it is generally okay for the routine to just return an error rather than going out of the way to free everything. Our feeling was that on a modern machine, it you get an allocation failure, you are probably going to be in some trouble, so code simplicity was more important. (It's okay to clean up, certainly, but not required so long as no outputs are left in an inconsistent state.)

With that said, I'd probably write this routine like:

```c
int git_cred_ssh_keyfile_passphrase_new(
        git_cred **cred,
        const char *publickey,
        const char *privatekey,
        const char *passphrase)
{
        git_cred_ssh_keyfile_passphrase *c;

        assert(cred && privatekey);

        c = git__calloc(1, sizeof(git_cred_ssh_keyfile_passphrase));
        GITERR_CHECK_ALLOC(c);

        c->parent.credtype = GIT_CREDTYPE_SSH_KEYFILE_PASSPHRASE;
        c->parent.free = ssh_keyfile_passphrase_free;

        c->privatekey = git__strdup(privatekey);
        GITERR_CHECK_ALLOC(c->privatekey);

        if (publickey) {
                c->publickey = git__strdup(publickey);
                GITERR_CHECK_ALLOC(c->publickey);
        }

        if (passphrase) {
                c->passphrase = git__strdup(passphrase);
                GITERR_CHECK_ALLOC(c->passphrase);
        }

        *cred = (git_cred *)c;
        return 0;
}
```

By using `git__calloc`, I ensure that the structure is already cleared to zero, so you don't need to `else` clauses when the publickey and passphrase are passed in as NULL. It also future proofs against forgetting to initialize any other fields.

I check that the `cred` and `privatekey` parameters are passed in validly with an assert, which is the standard in the library for checks that are just protecting against obvious programmer error. Previously there was an `if` to check `cred` and no check on `privatekey`.

Using the `GITERR_CHECK_ALLOC()` macro does mean that the earlier allocations in the function will be leaked if an allocation failure happens, but as I mentioned, it is allowed by the coding rules of libgit2 and makes the function easier to read. Also, since `cred` is not written to until the end of the function, there is no chance of a partially initialized and invalid structure being leaked to the caller even in the cases of failure.

I know this is style stuff as opposed to the functional content of your work; I don't mean to get caught up in minor issues, but for the maintainability of the library, the style stuff matters to us, too.