git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH] Add a simple getpass() for MinGW
Date: Thu, 09 Apr 2009 21:48:48 +0200	[thread overview]
Message-ID: <49DE5120.8050904@kdbg.org> (raw)
In-Reply-To: <7ba615a300fe2742e8d32f0313c6ee9a1a1aaed3.1239154140u.git.johannes.schindelin@gmx.de>

Johannes Schindelin schrieb:
> This should be replaced with a graphical getpass() at some stage.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 
> 	I saw it coming that I had to do this.

There are two callers of getpass: One is in imap-send.c, but we don't 
build it on Windows. The other is in http.c. But notice that this is only 
built if NO_CURL is not defined, yet, upstream git defines it in the MinGW 
section, and so this patch alone is not needed in upstream git.

I see you have removed NO_CURL = YesPlease in 4msysgit.git. You should 
make it a part of a series that removes NO_CURL = YesPlease from the MinGW 
section.

>  compat/mingw.c |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index d50186e..2ab5bbe 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1157,3 +1157,18 @@ int link(const char *oldpath, const char *newpath)
>  	}
>  	return 0;
>  }
> +
> +char *getpass(const char *prompt)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +
> +	fputs(prompt, stderr);
> +	for (;;) {
> +		char c = _getch();
> +		if (c == '\r' || c == '\n')
> +			break;
> +		strbuf_addch(&buf, c);
> +	}
> +	fputs("\n", stderr);
> +	return strbuf_detach(&buf, NULL);
> +}

Where do the callers get the prototype from (on MinGW)? Usually, we have 
to have a corresponding function declaration in compat/mingw.h for 
functions that are missing on Windows.

 From http://opengroup.org/onlinepubs/007908775/xsh/getpass.html:

   The return value points to static data whose content may be overwritten
   by each call.

I'm not saying that you should use a fixed-size static character array, 
but only that you should not leak memory on each call ;) (But not even 
that is very important; I'm just summarizing the research I did because I 
was wondering what would happen to the returned buffer.)

Apart from that, the implementation looks good. (_getch(), according to 
the docs on MSDN, doesn't echo the input.)

-- Hannes

  parent reply	other threads:[~2009-04-09 19:50 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1239154140u.git.johannes.schindelin@gmx.de>
2009-04-08  1:30 ` [PATCH] Add a simple getpass() for MinGW Johannes Schindelin
2009-04-08  2:32   ` Junio C Hamano
2009-04-08  2:56     ` Johannes Schindelin
2009-04-09 19:48   ` Johannes Sixt [this message]
2009-04-10 18:03     ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49DE5120.8050904@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).