"Fabian Stelzer via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > From: Fabian Stelzer <fs@xxxxxxxxxxxx> > > calls ssh-add -L and uses the first key Documentation/SubmittingPatches::[[describe-changes]]. > Signed-off-by: Fabian Stelzer <fs@xxxxxxxxxxxx> > --- > gpg-interface.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/gpg-interface.c b/gpg-interface.c > index 3c9a48c8e7e..c956ed87475 100644 > --- a/gpg-interface.c > +++ b/gpg-interface.c > @@ -467,6 +467,23 @@ int git_gpg_config(const char *var, const char *value, void *cb) > return 0; > } > > +/* Returns the first public key from an ssh-agent to use for signing */ > +static char *get_default_ssh_signing_key(void) { Style. Open and close braces around a function sit on their own lines by themselves. > + struct child_process ssh_add = CHILD_PROCESS_INIT; > + int ret = -1; > + struct strbuf key_stdout = STRBUF_INIT; > + struct strbuf **keys; Whose releasing the resource held by "keys" when we return? > + strvec_pushl(&ssh_add.args, "ssh-add", "-L", NULL); > + ret = pipe_command(&ssh_add, NULL, 0, &key_stdout, 0, NULL, 0); I often load about half a dozen keys to my ssh-agent so "ssh-add -L" will give me multi-line output. I know you wrote "the first public key" above, but that does not mean users who needs to have multiple keys can be limited to use only the first key for signing. There should be a way to say "I may have many keys for other reasons, but for signing I want to use this key, not the other ones". > + if (!ret) { > + keys = strbuf_split_max(&key_stdout, '\n', 2); Let's not use strbuf_split_*() that is a horribly wrong interface. You do not want a set of elastic buffer after splitting. You only are peeking the first line, no? You are leaking keys[] array and probably keys[1], too. eol = strchrnul(key_stdout.buf, '\n'); strbuf_setlen(&key_stdout, eol - key_stdout.buf); or something along that line, perhaps? > + if (keys[0]) > + return strbuf_detach(keys[0], NULL); > + } > + > + return ""; > +} > const char *get_signing_key(void) Missing blank line after the function body. > { > if (configured_signing_key)