From: Junio C Hamano Date: Wed, 03 Mar 2010 19:56:20 GMT Subject: Re: [PATCH v4 2/3] git-core: Support retrieving passwords with GIT_ASKPASS Message-ID: <7vd3zlqivv.fsf@alter.siamese.dyndns.org> In-Reply-To: <1267616208-3844-1-git-send-email-lznuaa@gmail.com> Frank Li writes: > imap-send and authority http connect reads passwords from an interactive > terminal. This behavious cause GUIs to hang waiting for git complete. > > Fix this problem by allowing a password-retrieving command > to be specified in GIT_ASKPASS > > Signed-off-by: Frank Li > --- It appears that between v3 and v4 you incorporated all the changes J6t suggested. I would have preferred to see you Cc him, so that he can simply respond with "Acked-by:" (or another round of review cycle). > +char *git_getpass(const char *prompt) > +{ > ... > + memset(&pass, 0, sizeof(pass)); > + pass.argv = args; > + pass.out = -1; > + > + if (start_command(&pass)) > + exit(1); I understand that you exit silently here because start-command would have given diagnostic messages already. > + if (strbuf_read(&buffer, pass.out, 20) < 0) > + exit(1); What about this one? Running out of memory inside strbuf_grow() may have died with message but other error conditions that do give control back to the caller seem pretty quiet to my cursory look... > + close(pass.out); > + > + if (finish_command(&pass)) > + exit(1); This silent exit is correct, I think; wait_or_whine() called by finish_command() would have reported errors. > + strbuf_setlen(&buffer, strcspn(buffer.buf, "\r\n")); > + > + /*it maybe memory leak because getpass return a static buffer*/ > + return strbuf_detach(&buffer, NULL); Perhaps keep this value in a "static char *" in the function and free it upon entry on the second call?