Re: [PATCH] add Android support

From: Junio C Hamano <gitster@pobox.com>
Date: May 16, 2011, 21:11 UTC
Message-ID: <7vzkmmjqpx.fsf@alter.siamese.dyndns.org>
In-Reply-To: <D2F37FDB-20C2-401A-AA90-DB4A50C27123@gieschke.de>

Rafael Gieschke <rafael@gieschke.de> writes:

Show 34 quoted lines

> diff --git a/connect.c b/connect.c
> index 57dc20c..15b285e 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -632,7 +632,11 @@ char *git_getpass(const char *prompt)
>  	if (!askpass)
>  		askpass = getenv("SSH_ASKPASS");
>  	if (!askpass || !(*askpass)) {
> +		#ifndef NO_GETPASS
>  		char *result = getpass(prompt);
> +		#else
> +		char *result = NULL;
> +		#endif
>  		if (!result)
>  			die_errno("Could not read password");
>  		return result;
> diff --git a/ident.c b/ident.c
> index 1c4adb0..76fa786 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -20,7 +20,12 @@ static void copy_gecos(const struct passwd *w, char *name, size_t sz)
>  	 * with commas.  Also & stands for capitalized form of the login name.
>  	 */
>  
> -	for (len = 0, dst = name, src = w->pw_gecos; len < sz; src++) {
> +	#ifndef NO_PW_GECOS
> +	src = w->pw_gecos;
> +	#else
> +	src = "&";
> +	#endif
> +	for (len = 0, dst = name; len < sz; src++) {
>  		int ch = *src;
>  		if (ch != '&') {
>  			*dst++ = ch;

Please do not throw in conditional compilation in a codepath that is otherwise generic.

Do something like this near the beginning of the file (or if they are common, in an appropriate header):

        #ifdef NO_GETPASS
        #define getpass(ignored) NULL
        #endif

        #ifdef NO_PW_GECOS
        #define get_gecos(ignored) "&"
        #else
        #define get_gecos(struct_passwd) (struct_passwd->pw_gecos)
        #endif

That way, you do not have to change connect.c at all, and the code that accesses gecos field would get a slight abstraction, i.e.

	for (len = 0, dst = name, src = get_gecos(w); len < sz; src++) {
		...

I however suspect that NO_GETPASS would be a useless thing in the longer term. Wouldn't you rather wish to have a native Android UI that asks a password and plug that implementation as a replacement for git_getpass()?

It might be worthwhile to study how mingw folks do this part before you dive in and butcher this codepath in a way you may regret later.