[PATCH] Add a simple getpass() for MinGW

[PATCH] Add a simple getpass() for MinGW

[Johannes Schindelin]

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.

 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);
+}
-- 
1.6.2.1.613.g25746

Re: [PATCH] Add a simple getpass() for MinGW

[Junio C Hamano]

Thanks; should I take this directly or via Hannes's pull request?

Re: [PATCH] Add a simple getpass() for MinGW

[Johannes Schindelin]

Hi,

On Tue, 7 Apr 2009, Junio C Hamano wrote:

> Thanks; should I take this directly or via Hannes's pull request?

Thanks for asking; Maybe Hannes has some alternative solution, so I'd like 
to have his take on things first.

Ciao,
Dscho "who really, really needs to sleep now"

Re: [PATCH] Add a simple getpass() for MinGW

[Johannes Sixt]

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

Re: [PATCH] Add a simple getpass() for MinGW

[Johannes Schindelin]

Hi,

On Thu, 9 Apr 2009, Johannes Sixt wrote:

> 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.

Indeed we did, as we ship curl, and we rely on http:// protocol being 
available in the netinstaller.

> >  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.)

Good catch!

Will fix.

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

I'll probably add handling for ^H (as I need it very often ;-), but not 
for ^W (overkill).

Ciao,
Dscho