All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.