[PATCH] contrib: GnomeKeyring support + generic helper implementation

[PATCH] contrib: GnomeKeyring support + generic helper implementation

[Philipp A. Hartmann]

All,

the following patch series proposes enhancements to the credential helper
implementations in the contrib section.  The detailed development history
can be found at GitHub [1].

  The first patch adds a GnomeKeyring credential backend.  The GnomeKeyring
specific parts are based on the work by John Szakmeister, who wrote the
helper originally for the initial, unpublished version of the credential
helper protocol.

  The second patch adds a generic implementation of a credential helper
based on a simplified credential API and common helper functions.  Helpers
based on this implementation do not need to worry about the credential
protocol and only need to implement callback functions for the different
credential operations.

  The third and fourth patches port the existing helpers to this generic
implementation.

Looking forward to your thoughts.

Greetings from Oldenburg,
  Philipp

[1] https://github.com/pah/git-credential-helper

[PATCH 1/4] contrib: add credential helper for GnomeKeyring

[Philipp A. Hartmann]

From: "Philipp A. Hartmann" <pah@qo.cx>

With this installed in your $PATH, you can store
git-over-http passwords in your keyring by doing:

git config credential.helper gnome-keyring

The code is based in large part on the work of John Szakmeister
who wrote the helper originally for the initial, unpublished
version of the credential helper protocol.

This version will pass t0303 if you do:

  GIT_TEST_CREDENTIAL_HELPER=gnome-keyring \
  ./t0303-credential-external.sh

Signed-off-by: Philipp A. Hartmann <pah@qo.cx>
---
 contrib/credential/gnome-keyring/.gitignore        |    1 +
 contrib/credential/gnome-keyring/Makefile          |   24 ++
 .../gnome-keyring/git-credential-gnome-keyring.c   |  445 ++++++++++++++++++++
 3 files changed, 470 insertions(+)
 create mode 100644 contrib/credential/gnome-keyring/.gitignore
 create mode 100644 contrib/credential/gnome-keyring/Makefile
 create mode 100644 contrib/credential/gnome-keyring/git-credential-gnome-keyring.c

diff --git a/contrib/credential/gnome-keyring/.gitignore b/contrib/credential/gnome-keyring/.gitignore
new file mode 100644
index 0000000..88d8fcd
--- /dev/null
+++ b/contrib/credential/gnome-keyring/.gitignore
@@ -0,0 +1 @@
+git-credential-gnome-keyring
diff --git a/contrib/credential/gnome-keyring/Makefile b/contrib/credential/gnome-keyring/Makefile
new file mode 100644
index 0000000..e6561d8
--- /dev/null
+++ b/contrib/credential/gnome-keyring/Makefile
@@ -0,0 +1,24 @@
+MAIN:=git-credential-gnome-keyring
+all:: $(MAIN)
+
+CC = gcc
+RM = rm -f
+CFLAGS = -g -O2 -Wall
+
+-include ../../../config.mak.autogen
+-include ../../../config.mak
+
+INCS:=$(shell pkg-config --cflags gnome-keyring-1)
+LIBS:=$(shell pkg-config --libs gnome-keyring-1)
+
+SRCS:=$(MAIN).c
+OBJS:=$(SRCS:.c=.o)
+
+%.o: %.c
+	$(CC) $(CFLAGS) $(CPPFLAGS) $(INCS) -o $@ -c $<
+
+$(MAIN): $(OBJS)
+	$(CC) -o $@ $(LDFLAGS) $^ $(LIBS)
+
+clean:
+	@$(RM) $(MAIN) $(OBJS)
diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
new file mode 100644
index 0000000..41f61c5
--- /dev/null
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -0,0 +1,445 @@
+/*
+ * Copyright (C) 2011 John Szakmeister <john@szakmeister.net>
+ *               2012 Philipp A. Hartmann <pah@qo.cx>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+/*
+ * Credits:
+ * - GNOME Keyring API handling originally written by John Szakmeister
+ * - ported to credential helper API by Philipp A. Hartmann
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdarg.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <gnome-keyring.h>
+
+/*
+ * This credential struct and API is simplified from git's credential.{h,c}
+ */
+struct credential
+{
+	char          *protocol;
+	char          *host;
+	unsigned short port;
+	char          *path;
+	char          *username;
+	char          *password;
+};
+
+#define CREDENTIAL_INIT \
+  { NULL,NULL,0,NULL,NULL,NULL }
+
+void credential_init(struct credential *c);
+void credential_clear(struct credential *c);
+int  credential_read(struct credential *c);
+void credential_write(const struct credential *c);
+
+typedef int (*credential_op_cb)(struct credential*);
+
+struct credential_operation
+{
+	char             *name;
+	credential_op_cb op;
+};
+
+#define CREDENTIAL_OP_END \
+  { NULL,NULL }
+
+/*
+ * Table with operation callbacks is defined in concrete
+ * credential helper implementation and contains entries
+ * like { "get", function_to_get_credential } terminated
+ * by CREDENTIAL_OP_END.
+ */
+struct credential_operation const credential_helper_ops[];
+
+/* ---------------- common helper functions ----------------- */
+
+static inline void free_password(char *password)
+{
+	char *c = password;
+	if (!password)
+		return;
+
+	while (*c) *c++ = '\0';
+	free(password);
+}
+
+static inline void warning(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	fprintf(stderr, "warning: ");
+	vfprintf(stderr, fmt, ap);
+	fprintf(stderr, "\n" );
+	va_end(ap);
+}
+
+static inline void error(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	fprintf(stderr, "error: ");
+	vfprintf(stderr, fmt, ap);
+	fprintf(stderr, "\n" );
+	va_end(ap);
+}
+
+static inline void die(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap,fmt);
+	error(fmt, ap);
+	va_end(ap);
+	exit(EXIT_FAILURE);
+}
+
+static inline void die_errno(int err)
+{
+	error("%s", strerror(err));
+	exit(EXIT_FAILURE);
+}
+
+static inline char *xstrdup(const char *str)
+{
+	char *ret = strdup(str);
+	if (!ret)
+		die_errno(errno);
+
+	return ret;
+}
+
+/* ----------------- GNOME Keyring functions ----------------- */
+
+/* create a special keyring option string, if path is given */
+static char* keyring_object(struct credential *c)
+{
+	char* object = NULL;
+
+	if (!c->path)
+		return object;
+
+	object = (char*) malloc(strlen(c->host)+strlen(c->path)+8);
+	if(!object)
+		die_errno(errno);
+
+	if(c->port)
+		sprintf(object,"%s:%hd/%s",c->host,c->port,c->path);
+	else
+		sprintf(object,"%s/%s",c->host,c->path);
+
+	return object;
+}
+
+int keyring_get(struct credential *c)
+{
+	char* object = NULL;
+	GList *entries;
+	GnomeKeyringNetworkPasswordData *password_data;
+	GnomeKeyringResult result;
+
+	if (!c->protocol || !(c->host || c->path))
+		return EXIT_FAILURE;
+
+	object = keyring_object(c);
+
+	result = gnome_keyring_find_network_password_sync(
+				c->username,
+				NULL /* domain */,
+				c->host,
+				object,
+				c->protocol,
+				NULL /* authtype */,
+				c->port,
+				&entries);
+
+	free(object);
+
+	if (result == GNOME_KEYRING_RESULT_NO_MATCH)
+		return EXIT_SUCCESS;
+
+	if (result == GNOME_KEYRING_RESULT_CANCELLED)
+		return EXIT_SUCCESS;
+
+	if (result != GNOME_KEYRING_RESULT_OK) {
+		error("%s",gnome_keyring_result_to_message(result));
+		return EXIT_FAILURE;
+	}
+
+	/* pick the first one from the list */
+	password_data = (GnomeKeyringNetworkPasswordData *) entries->data;
+
+	free_password(c->password);
+	c->password = xstrdup(password_data->password);
+
+	if (!c->username)
+		c->username = xstrdup(password_data->user);
+
+	gnome_keyring_network_password_list_free(entries);
+
+	return EXIT_SUCCESS;
+}
+
+
+int keyring_store(struct credential *c)
+{
+	guint32 item_id;
+	char  *object = NULL;
+
+	/*
+	 * Sanity check that what we are storing is actually sensible.
+	 * In particular, we can't make a URL without a protocol field.
+	 * Without either a host or pathname (depending on the scheme),
+	 * we have no primary key. And without a username and password,
+	 * we are not actually storing a credential.
+	 */
+	if (!c->protocol || !(c->host || c->path) ||
+	    !c->username || !c->password)
+		return EXIT_FAILURE;
+
+	object = keyring_object(c);
+
+	gnome_keyring_set_network_password_sync(
+				GNOME_KEYRING_DEFAULT,
+				c->username,
+				NULL /* domain */,
+				c->host,
+				object,
+				c->protocol,
+				NULL /* authtype */,
+				c->port,
+				c->password,
+				&item_id);
+
+	free(object);
+	return EXIT_SUCCESS;
+}
+
+int keyring_erase(struct credential *c)
+{
+	char  *object = NULL;
+	GList *entries;
+	GnomeKeyringNetworkPasswordData *password_data;
+	GnomeKeyringResult result;
+
+	/*
+	 * Sanity check that we actually have something to match
+	 * against. The input we get is a restrictive pattern,
+	 * so technically a blank credential means "erase everything".
+	 * But it is too easy to accidentally send this, since it is equivalent
+	 * to empty input. So explicitly disallow it, and require that the
+	 * pattern have some actual content to match.
+	 */
+	if (!c->protocol && !c->host && !c->path && !c->username)
+		return EXIT_FAILURE;
+
+	object = keyring_object(c);
+
+	result = gnome_keyring_find_network_password_sync(
+				c->username,
+				NULL /* domain */,
+				c->host,
+				object,
+				c->protocol,
+				NULL /* authtype */,
+				c->port,
+				&entries);
+
+	free(object);
+
+	if (result == GNOME_KEYRING_RESULT_NO_MATCH)
+		return EXIT_SUCCESS;
+
+	if (result == GNOME_KEYRING_RESULT_CANCELLED)
+		return EXIT_SUCCESS;
+
+	if (result != GNOME_KEYRING_RESULT_OK)
+	{
+		error("%s",gnome_keyring_result_to_message(result));
+		return EXIT_FAILURE;
+	}
+
+	/* pick the first one from the list (delete all matches?) */
+	password_data = (GnomeKeyringNetworkPasswordData *) entries->data;
+
+	result = gnome_keyring_item_delete_sync(
+		password_data->keyring, password_data->item_id);
+
+	gnome_keyring_network_password_list_free(entries);
+
+	if (result != GNOME_KEYRING_RESULT_OK)
+	{
+		error("%s",gnome_keyring_result_to_message(result));
+		return EXIT_FAILURE;
+	}
+
+	return EXIT_SUCCESS;
+}
+
+/*
+ * Table with helper operation callbacks, used by generic
+ * credential helper main function.
+ */
+struct credential_operation const credential_helper_ops[] =
+{
+	{ "get",   keyring_get   },
+	{ "store", keyring_store },
+	{ "erase", keyring_erase },
+	CREDENTIAL_OP_END
+};
+
+/* ------------------ credential functions ------------------ */
+
+void credential_init(struct credential *c)
+{
+	memset(c, 0, sizeof(*c));
+}
+
+void credential_clear(struct credential *c)
+{
+	free(c->protocol);
+	free(c->host);
+	free(c->path);
+	free(c->username);
+	free_password(c->password);
+
+	credential_init(c);
+}
+
+int credential_read(struct credential *c)
+{
+	char    buf[1024];
+	ssize_t line_len = 0;
+	char   *key      = buf;
+	char   *value;
+
+	while (fgets(buf, sizeof(buf), stdin))
+	{
+		line_len = strlen(buf);
+
+		if(buf[line_len-1]=='\n')
+			buf[--line_len]='\0';
+
+		if(!line_len)
+			break;
+
+		value = strchr(buf,'=');
+		if(!value) {
+			warning("invalid credential line: %s", key);
+			return -1;
+		}
+		*value++ = '\0';
+
+		if (!strcmp(key, "protocol")) {
+			free(c->protocol);
+			c->protocol = xstrdup(value);
+		} else if (!strcmp(key, "host")) {
+			free(c->host);
+			c->host = xstrdup(value);
+			value = strrchr(c->host,':');
+			if (value) {
+				*value++ = '\0';
+				c->port = atoi(value);
+			}
+		} else if (!strcmp(key, "path")) {
+			free(c->path);
+			c->path = xstrdup(value);
+		} else if (!strcmp(key, "username")) {
+			free(c->username);
+			c->username = xstrdup(value);
+		} else if (!strcmp(key, "password")) {
+			free_password(c->password);
+			c->password = xstrdup(value);
+			while (*value) *value++ = '\0';
+		}
+		/*
+		 * Ignore other lines; we don't know what they mean, but
+		 * this future-proofs us when later versions of git do
+		 * learn new lines, and the helpers are updated to match.
+		 */
+	}
+	return 0;
+}
+
+void credential_write_item(FILE *fp, const char *key, const char *value)
+{
+	if (!value)
+		return;
+	fprintf(fp, "%s=%s\n", key, value);
+}
+
+void credential_write(const struct credential *c)
+{
+	/* only write username/password, if set */
+	credential_write_item(stdout, "username", c->username);
+	credential_write_item(stdout, "password", c->password);
+}
+
+static void usage(const char *name)
+{
+	struct credential_operation const *try_op = credential_helper_ops;
+	const char *basename = strrchr(name,'/');
+
+	basename = (basename) ? basename + 1 : name;
+	fprintf(stderr, "Usage: %s <", basename);
+	while(try_op->name) {
+		fprintf(stderr,"%s",(try_op++)->name);
+		if(try_op->name)
+			fprintf(stderr,"%s","|");
+	}
+	fprintf(stderr,"%s",">\n");
+}
+
+int main(int argc, char *argv[])
+{
+	int ret = EXIT_SUCCESS;
+
+	struct credential_operation const *try_op = credential_helper_ops;
+	struct credential                  cred   = CREDENTIAL_INIT;
+
+	if (!argv[1]) {
+		usage(argv[0]);
+		goto out;
+	}
+
+	/* lookup operation callback */
+	while(try_op->name && strcmp(argv[1], try_op->name))
+		try_op++;
+
+	/* unsupported operation given -- ignore silently */
+	if(!try_op->name || !try_op->op)
+		goto out;
+
+	ret = credential_read(&cred);
+	if(ret)
+		goto out;
+
+	/* perform credential operation */
+	ret = (*try_op->op)(&cred);
+
+	credential_write(&cred);
+
+out:
+	credential_clear(&cred);
+	return ret;
+}
-- 
1.7.10.4

[PATCH 2/4] contrib: add generic credential helper

[Philipp A. Hartmann]

From: "Philipp A. Hartmann" <pah@qo.cx>

This adds a generic implementation for credential helpers.

It provides a header file credential_helper.h containing
a simplified credential API and common helper functions.
The implementation in credential_helper.c already provides
a main() function and chooses the credential operation
from a function table:

  struct credential_operation const credential_helper_ops[] =
  {
      { "get",   my_backend_get   },
      { "store", my_backend_store },
      { "erase", my_backend_erase },
      CREDENTIAL_OP_END
  };

With this, a specific credential helper backend usually only
needs to implement these operation callbacks.

Signed-off-by: Philipp A. Hartmann <pah@qo.cx>
---
 contrib/credential/helper/credential_helper.c |  149 +++++++++++++++++++++++++
 contrib/credential/helper/credential_helper.h |  116 +++++++++++++++++++
 2 files changed, 265 insertions(+)
 create mode 100644 contrib/credential/helper/credential_helper.c
 create mode 100644 contrib/credential/helper/credential_helper.h

diff --git a/contrib/credential/helper/credential_helper.c b/contrib/credential/helper/credential_helper.c
new file mode 100644
index 0000000..e99c2ec
--- /dev/null
+++ b/contrib/credential/helper/credential_helper.c
@@ -0,0 +1,149 @@
+/*
+ * Copyright (C) 2012 Philipp A. Hartmann <pah@qo.cx>
+ *
+ * This file is licensed under the GPL v2, or a later version
+ * at the discretion of Linus.
+ *
+ * This credential struct and API is simplified from git's
+ * credential.{h,c} to be used within credential helper
+ * implementations.
+ */
+
+#include <credential_helper.h>
+
+void credential_init(struct credential *c)
+{
+	memset(c, 0, sizeof(*c));
+}
+
+void credential_clear(struct credential *c)
+{
+	free(c->protocol);
+	free(c->host);
+	free(c->path);
+	free(c->username);
+	free_password(c->password);
+
+	credential_init(c);
+}
+
+int credential_read(struct credential *c)
+{
+	char    buf[1024];
+	ssize_t line_len = 0;
+	char   *key      = buf;
+	char   *value;
+
+	while (fgets(buf, sizeof(buf), stdin))
+	{
+		line_len = strlen(buf);
+
+		if(buf[line_len-1]=='\n')
+			buf[--line_len]='\0';
+
+		if(!line_len)
+			break;
+
+		value = strchr(buf,'=');
+		if(!value) {
+			warning("invalid credential line: %s", key);
+			return -1;
+		}
+		*value++ = '\0';
+
+		if (!strcmp(key, "protocol")) {
+			free(c->protocol);
+			c->protocol = xstrdup(value);
+		} else if (!strcmp(key, "host")) {
+			free(c->host);
+			c->host = xstrdup(value);
+			value = strrchr(c->host,':');
+			if (value) {
+				*value++ = '\0';
+				c->port = atoi(value);
+			}
+		} else if (!strcmp(key, "path")) {
+			free(c->path);
+			c->path = xstrdup(value);
+		} else if (!strcmp(key, "username")) {
+			free(c->username);
+			c->username = xstrdup(value);
+		} else if (!strcmp(key, "password")) {
+			free_password(c->password);
+			c->password = xstrdup(value);
+			while (*value) *value++ = '\0';
+		}
+		/*
+		 * Ignore other lines; we don't know what they mean, but
+		 * this future-proofs us when later versions of git do
+		 * learn new lines, and the helpers are updated to match.
+		 */
+	}
+	return 0;
+}
+
+void credential_write_item(FILE *fp, const char *key, const char *value)
+{
+	if (!value)
+		return;
+	fprintf(fp, "%s=%s\n", key, value);
+}
+
+void credential_write(const struct credential *c)
+{
+	/* only write username/password, if set */
+	credential_write_item(stdout, "username", c->username);
+	credential_write_item(stdout, "password", c->password);
+}
+
+static void usage(const char *name)
+{
+	struct credential_operation const *try_op = credential_helper_ops;
+	const char *basename = strrchr(name,'/');
+
+	basename = (basename) ? basename + 1 : name;
+	fprintf(stderr, "Usage: %s <", basename);
+	while(try_op->name) {
+		fprintf(stderr,"%s",(try_op++)->name);
+		if(try_op->name)
+			fprintf(stderr,"%s","|");
+	}
+	fprintf(stderr,"%s",">\n");
+}
+
+/*
+ * generic main function for credential helpers
+ */
+int main(int argc, char *argv[])
+{
+	int ret = EXIT_SUCCESS;
+
+	struct credential_operation const *try_op = credential_helper_ops;
+	struct credential                  cred   = CREDENTIAL_INIT;
+
+	if (!argv[1]) {
+		usage(argv[0]);
+		goto out;
+	}
+
+	/* lookup operation callback */
+	while(try_op->name && strcmp(argv[1], try_op->name))
+		try_op++;
+
+	/* unsupported operation given -- ignore silently */
+	if(!try_op->name || !try_op->op)
+		goto out;
+
+	ret = credential_read(&cred);
+	if(ret)
+		goto out;
+
+	/* perform credential operation */
+	ret = (*try_op->op)(&cred);
+
+	credential_write(&cred);
+
+out:
+	credential_clear(&cred);
+	return ret;
+}
diff --git a/contrib/credential/helper/credential_helper.h b/contrib/credential/helper/credential_helper.h
new file mode 100644
index 0000000..8266078
--- /dev/null
+++ b/contrib/credential/helper/credential_helper.h
@@ -0,0 +1,116 @@
+/*
+ * Copyright (C) 2012 Philipp A. Hartmann <pah@qo.cx>
+ *
+ * This file is licensed under the GPL v2, or a later version
+ * at the discretion of Linus.
+ *
+ * This credential struct and API is simplified from git's
+ * credential.{h,c} to be used within credential helper
+ * implementations.
+ */
+#ifndef CREDENTIAL_HELPER_H_INCLUDED_
+#define CREDENTIAL_HELPER_H_INCLUDED_
+
+#include <stdio.h>
+#include <string.h>
+#include <stdarg.h>
+#include <stdlib.h>
+#include <errno.h>
+
+struct credential
+{
+	char          *protocol;
+	char          *host;
+	unsigned short port;
+	char          *path;
+	char          *username;
+	char          *password;
+};
+
+#define CREDENTIAL_INIT \
+  { NULL,NULL,0,NULL,NULL,NULL }
+
+void credential_init(struct credential *c);
+void credential_clear(struct credential *c);
+int  credential_read(struct credential *c);
+void credential_write(const struct credential *c);
+
+typedef int (*credential_op_cb)(struct credential*);
+
+struct credential_operation
+{
+	char             *name;
+	credential_op_cb op;
+};
+
+#define CREDENTIAL_OP_END \
+  { NULL,NULL }
+
+/*
+ * Table with operation callbacks is defined in concrete
+ * credential helper implementation and contains entries
+ * like { "get", function_to_get_credential } terminated
+ * by CREDENTIAL_OP_END.
+ */
+extern struct credential_operation const credential_helper_ops[];
+
+/* ---------------- common helper functions ---------------- */
+
+static inline void free_password(char *password)
+{
+	char *c = password;
+	if (!password)
+		return;
+
+	while (*c) *c++ = '\0';
+	free(password);
+}
+
+static inline void warning(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	fprintf(stderr, "warning: ");
+	vfprintf(stderr, fmt, ap);
+	fprintf(stderr, "\n" );
+	va_end(ap);
+}
+
+static inline void error(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	fprintf(stderr, "error: ");
+	vfprintf(stderr, fmt, ap);
+	fprintf(stderr, "\n" );
+	va_end(ap);
+}
+
+static inline void die(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap,fmt);
+	error(fmt, ap);
+	va_end(ap);
+	exit(EXIT_FAILURE);
+}
+
+static inline void die_errno(int err)
+{
+	error("%s", strerror(err));
+	exit(EXIT_FAILURE);
+}
+
+static inline char *xstrdup(const char *str)
+{
+	char *ret = strdup(str);
+	if (!ret)
+		die_errno(errno);
+
+	return ret;
+}
+
+#endif /* CREDENTIAL_HELPER_H_INCLUDED_ */
-- 
1.7.10.4

[PATCH 3/4] gnome-keyring: port to generic helper implementation

[Philipp A. Hartmann]

From: "Philipp A. Hartmann" <pah@qo.cx>

Use generic credential helper implementation in the
GnomeKeyring credential helper.

The GnomeKeyring helper has been using the generic implementation
internally already and therefore only drops the duplicate code.

Signed-off-by: Philipp A. Hartmann <pah@qo.cx>
---
 contrib/credential/gnome-keyring/Makefile          |    6 +-
 .../gnome-keyring/git-credential-gnome-keyring.c   |  243 +-------------------
 2 files changed, 6 insertions(+), 243 deletions(-)

diff --git a/contrib/credential/gnome-keyring/Makefile b/contrib/credential/gnome-keyring/Makefile
index e6561d8..7f3ec11 100644
--- a/contrib/credential/gnome-keyring/Makefile
+++ b/contrib/credential/gnome-keyring/Makefile
@@ -11,11 +11,15 @@ CFLAGS = -g -O2 -Wall
 INCS:=$(shell pkg-config --cflags gnome-keyring-1)
 LIBS:=$(shell pkg-config --libs gnome-keyring-1)
 
+HELPER:=../helper
+VPATH +=$(HELPER)
+
 SRCS:=$(MAIN).c
+SRCS+=credential_helper.c
 OBJS:=$(SRCS:.c=.o)
 
 %.o: %.c
-	$(CC) $(CFLAGS) $(CPPFLAGS) $(INCS) -o $@ -c $<
+	$(CC) $(CFLAGS) $(CPPFLAGS) -I$(HELPER) $(INCS) -o $@ -c $<
 
 $(MAIN): $(OBJS)
 	$(CC) -o $@ $(LDFLAGS) $^ $(LIBS)
diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 41f61c5..00244aa 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -23,114 +23,9 @@
  * - ported to credential helper API by Philipp A. Hartmann
  */
 
-#include <stdio.h>
-#include <string.h>
-#include <stdarg.h>
-#include <stdlib.h>
-#include <errno.h>
+#include <credential_helper.h>
 #include <gnome-keyring.h>
 
-/*
- * This credential struct and API is simplified from git's credential.{h,c}
- */
-struct credential
-{
-	char          *protocol;
-	char          *host;
-	unsigned short port;
-	char          *path;
-	char          *username;
-	char          *password;
-};
-
-#define CREDENTIAL_INIT \
-  { NULL,NULL,0,NULL,NULL,NULL }
-
-void credential_init(struct credential *c);
-void credential_clear(struct credential *c);
-int  credential_read(struct credential *c);
-void credential_write(const struct credential *c);
-
-typedef int (*credential_op_cb)(struct credential*);
-
-struct credential_operation
-{
-	char             *name;
-	credential_op_cb op;
-};
-
-#define CREDENTIAL_OP_END \
-  { NULL,NULL }
-
-/*
- * Table with operation callbacks is defined in concrete
- * credential helper implementation and contains entries
- * like { "get", function_to_get_credential } terminated
- * by CREDENTIAL_OP_END.
- */
-struct credential_operation const credential_helper_ops[];
-
-/* ---------------- common helper functions ----------------- */
-
-static inline void free_password(char *password)
-{
-	char *c = password;
-	if (!password)
-		return;
-
-	while (*c) *c++ = '\0';
-	free(password);
-}
-
-static inline void warning(const char *fmt, ...)
-{
-	va_list ap;
-
-	va_start(ap, fmt);
-	fprintf(stderr, "warning: ");
-	vfprintf(stderr, fmt, ap);
-	fprintf(stderr, "\n" );
-	va_end(ap);
-}
-
-static inline void error(const char *fmt, ...)
-{
-	va_list ap;
-
-	va_start(ap, fmt);
-	fprintf(stderr, "error: ");
-	vfprintf(stderr, fmt, ap);
-	fprintf(stderr, "\n" );
-	va_end(ap);
-}
-
-static inline void die(const char *fmt, ...)
-{
-	va_list ap;
-
-	va_start(ap,fmt);
-	error(fmt, ap);
-	va_end(ap);
-	exit(EXIT_FAILURE);
-}
-
-static inline void die_errno(int err)
-{
-	error("%s", strerror(err));
-	exit(EXIT_FAILURE);
-}
-
-static inline char *xstrdup(const char *str)
-{
-	char *ret = strdup(str);
-	if (!ret)
-		die_errno(errno);
-
-	return ret;
-}
-
-/* ----------------- GNOME Keyring functions ----------------- */
-
 /* create a special keyring option string, if path is given */
 static char* keyring_object(struct credential *c)
 {
@@ -307,139 +202,3 @@ struct credential_operation const credential_helper_ops[] =
 	{ "erase", keyring_erase },
 	CREDENTIAL_OP_END
 };
-
-/* ------------------ credential functions ------------------ */
-
-void credential_init(struct credential *c)
-{
-	memset(c, 0, sizeof(*c));
-}
-
-void credential_clear(struct credential *c)
-{
-	free(c->protocol);
-	free(c->host);
-	free(c->path);
-	free(c->username);
-	free_password(c->password);
-
-	credential_init(c);
-}
-
-int credential_read(struct credential *c)
-{
-	char    buf[1024];
-	ssize_t line_len = 0;
-	char   *key      = buf;
-	char   *value;
-
-	while (fgets(buf, sizeof(buf), stdin))
-	{
-		line_len = strlen(buf);
-
-		if(buf[line_len-1]=='\n')
-			buf[--line_len]='\0';
-
-		if(!line_len)
-			break;
-
-		value = strchr(buf,'=');
-		if(!value) {
-			warning("invalid credential line: %s", key);
-			return -1;
-		}
-		*value++ = '\0';
-
-		if (!strcmp(key, "protocol")) {
-			free(c->protocol);
-			c->protocol = xstrdup(value);
-		} else if (!strcmp(key, "host")) {
-			free(c->host);
-			c->host = xstrdup(value);
-			value = strrchr(c->host,':');
-			if (value) {
-				*value++ = '\0';
-				c->port = atoi(value);
-			}
-		} else if (!strcmp(key, "path")) {
-			free(c->path);
-			c->path = xstrdup(value);
-		} else if (!strcmp(key, "username")) {
-			free(c->username);
-			c->username = xstrdup(value);
-		} else if (!strcmp(key, "password")) {
-			free_password(c->password);
-			c->password = xstrdup(value);
-			while (*value) *value++ = '\0';
-		}
-		/*
-		 * Ignore other lines; we don't know what they mean, but
-		 * this future-proofs us when later versions of git do
-		 * learn new lines, and the helpers are updated to match.
-		 */
-	}
-	return 0;
-}
-
-void credential_write_item(FILE *fp, const char *key, const char *value)
-{
-	if (!value)
-		return;
-	fprintf(fp, "%s=%s\n", key, value);
-}
-
-void credential_write(const struct credential *c)
-{
-	/* only write username/password, if set */
-	credential_write_item(stdout, "username", c->username);
-	credential_write_item(stdout, "password", c->password);
-}
-
-static void usage(const char *name)
-{
-	struct credential_operation const *try_op = credential_helper_ops;
-	const char *basename = strrchr(name,'/');
-
-	basename = (basename) ? basename + 1 : name;
-	fprintf(stderr, "Usage: %s <", basename);
-	while(try_op->name) {
-		fprintf(stderr,"%s",(try_op++)->name);
-		if(try_op->name)
-			fprintf(stderr,"%s","|");
-	}
-	fprintf(stderr,"%s",">\n");
-}
-
-int main(int argc, char *argv[])
-{
-	int ret = EXIT_SUCCESS;
-
-	struct credential_operation const *try_op = credential_helper_ops;
-	struct credential                  cred   = CREDENTIAL_INIT;
-
-	if (!argv[1]) {
-		usage(argv[0]);
-		goto out;
-	}
-
-	/* lookup operation callback */
-	while(try_op->name && strcmp(argv[1], try_op->name))
-		try_op++;
-
-	/* unsupported operation given -- ignore silently */
-	if(!try_op->name || !try_op->op)
-		goto out;
-
-	ret = credential_read(&cred);
-	if(ret)
-		goto out;
-
-	/* perform credential operation */
-	ret = (*try_op->op)(&cred);
-
-	credential_write(&cred);
-
-out:
-	credential_clear(&cred);
-	return ret;
-}
-- 
1.7.10.4

[PATCH 4/4] osxkeychain: port to generic credential helper implementation

[Philipp A. Hartmann]

From: "Philipp A. Hartmann" <pah@qo.cx>

This reduces code duplication in the osxkeychain helper by
basing the implementation on the generic helper implementation.

Alongside, the return codes of the helper are tightened to be
more consistent in corner cases and the memory containing
cleartext passwords is explicitly cleared when possible.

Signed-off-by: Philipp A. Hartmann <pah@qo.cx>
---
 contrib/credential/helper/credential_helper.h      |    9 +
 contrib/credential/osxkeychain/Makefile            |   22 ++-
 .../osxkeychain/git-credential-osxkeychain.c       |  183 ++++++++------------
 3 files changed, 96 insertions(+), 118 deletions(-)

diff --git a/contrib/credential/helper/credential_helper.h b/contrib/credential/helper/credential_helper.h
index 8266078..76b6e50 100644
--- a/contrib/credential/helper/credential_helper.h
+++ b/contrib/credential/helper/credential_helper.h
@@ -113,4 +113,13 @@ static inline char *xstrdup(const char *str)
 	return ret;
 }
 
+static inline char *xstrndup(const char *str, size_t len)
+{
+	char *ret = strndup(str,len);
+	if (!ret)
+		die_errno(errno);
+
+	return ret;
+}
+
 #endif /* CREDENTIAL_HELPER_H_INCLUDED_ */
diff --git a/contrib/credential/osxkeychain/Makefile b/contrib/credential/osxkeychain/Makefile
index 4b3a08a..64ee7c5 100644
--- a/contrib/credential/osxkeychain/Makefile
+++ b/contrib/credential/osxkeychain/Makefile
@@ -1,4 +1,5 @@
-all:: git-credential-osxkeychain
+MAIN:=git-credential-osxkeychain
+all:: $(MAIN)
 
 CC = gcc
 RM = rm -f
@@ -7,11 +8,20 @@ CFLAGS = -g -O2 -Wall
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
-git-credential-osxkeychain: git-credential-osxkeychain.o
-	$(CC) $(CFLAGS) -o $@ $< $(LDFLAGS) -Wl,-framework -Wl,Security
+INCS:=
+LIBS:=-Wl,-framework -Wl,Security
+HELPER:=../helper
+VPATH +=$(HELPER)
 
-git-credential-osxkeychain.o: git-credential-osxkeychain.c
-	$(CC) -c $(CFLAGS) $<
+SRCS:=$(MAIN).c
+SRCS+=credential_helper.c
+OBJS:=$(SRCS:.c=.o)
+
+%.o: %.c
+	$(CC) $(CFLAGS) $(CPPFLAGS) -I$(HELPER) $(INCS) -o $@ -c $<
+
+$(MAIN): $(OBJS)
+	$(CC) -o $@ $(LDFLAGS) $^ $(LIBS)
 
 clean:
-	$(RM) git-credential-osxkeychain git-credential-osxkeychain.o
+	@$(RM) $(MAIN) $(OBJS)
diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
index 6beed12..60bd973 100644
--- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
+++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
@@ -1,53 +1,40 @@
+
+#include <credential_helper.h>
+
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
 #include <Security/Security.h>
 
 static SecProtocolType protocol;
-static char *host;
-static char *path;
-static char *username;
-static char *password;
-static UInt16 port;
-
-static void die(const char *err, ...)
-{
-	char msg[4096];
-	va_list params;
-	va_start(params, err);
-	vsnprintf(msg, sizeof(msg), err, params);
-	fprintf(stderr, "%s\n", msg);
-	va_end(params);
-	exit(1);
-}
-
-static void *xstrdup(const char *s1)
-{
-	void *ret = strdup(s1);
-	if (!ret)
-		die("Out of memory");
-	return ret;
-}
 
 #define KEYCHAIN_ITEM(x) (x ? strlen(x) : 0), x
-#define KEYCHAIN_ARGS \
+#define KEYCHAIN_ARGS(c) \
 	NULL, /* default keychain */ \
-	KEYCHAIN_ITEM(host), \
+	KEYCHAIN_ITEM(c->host), \
 	0, NULL, /* account domain */ \
-	KEYCHAIN_ITEM(username), \
-	KEYCHAIN_ITEM(path), \
-	port, \
+	KEYCHAIN_ITEM(c->username), \
+	KEYCHAIN_ITEM(c->path), \
+	(UInt16) c->port, \
 	protocol, \
 	kSecAuthenticationTypeDefault
 
-static void write_item(const char *what, const char *buf, int len)
+static int prepare_internet_password(struct credential *c)
 {
-	printf("%s=", what);
-	fwrite(buf, 1, len, stdout);
-	putchar('\n');
+	if (!c->protocol)
+		return -1;
+	else if (!strcmp(c->protocol, "https"))
+		protocol = kSecProtocolTypeHTTPS;
+	else if (!strcmp(c->protocol, "http"))
+		protocol = kSecProtocolTypeHTTP;
+	else /* we don't yet handle other protocols */
+		return -1;
+
+	return 0;
 }
 
-static void find_username_in_item(SecKeychainItemRef item)
+static void
+find_username_in_item(SecKeychainItemRef item, struct credential *c)
 {
 	SecKeychainAttributeList list;
 	SecKeychainAttribute attr;
@@ -59,27 +46,37 @@ static void find_username_in_item(SecKeychainItemRef item)
 	if (SecKeychainItemCopyContent(item, NULL, &list, NULL, NULL))
 		return;
 
-	write_item("username", attr.data, attr.length);
+	free(c->username);
+	c->username = xstrndup(attr.data, attr.length);
+
 	SecKeychainItemFreeContent(&list, NULL);
 }
 
-static void find_internet_password(void)
+static int find_internet_password(struct credential *c)
 {
 	void *buf;
 	UInt32 len;
 	SecKeychainItemRef item;
 
-	if (SecKeychainFindInternetPassword(KEYCHAIN_ARGS, &len, &buf, &item))
-		return;
+	/* Silently ignore unsupported protocols */
+	if (prepare_internet_password(c))
+		return EXIT_SUCCESS;
 
-	write_item("password", buf, len);
-	if (!username)
-		find_username_in_item(item);
+	if (SecKeychainFindInternetPassword(KEYCHAIN_ARGS(c), &len, &buf, &item))
+		return EXIT_SUCCESS;
+
+	free_password(c->password);
+	c->password = xstrndup(buf, len);
+	memset(buf,len,'\0');
+
+	if (!c->username)
+		find_username_in_item(item, c);
 
 	SecKeychainItemFreeContent(NULL, buf);
+	return EXIT_SUCCESS;
 }
 
-static void delete_internet_password(void)
+static int delete_internet_password(struct credential *c)
 {
 	SecKeychainItemRef item;
 
@@ -88,86 +85,48 @@ static void delete_internet_password(void)
 	 * will give us; if you want to do something more fancy, use the
 	 * Keychain manager.
 	 */
-	if (!protocol || !host)
-		return;
+	if (!c->protocol || !c->host)
+		return EXIT_FAILURE;
 
-	if (SecKeychainFindInternetPassword(KEYCHAIN_ARGS, 0, NULL, &item))
-		return;
+	/* Silently ignore unsupported protocols */
+	if (prepare_internet_password(c))
+		return EXIT_SUCCESS;
+
+	if (SecKeychainFindInternetPassword(KEYCHAIN_ARGS(c), 0, NULL, &item))
+		return EXIT_SUCCESS;
 
-	SecKeychainItemDelete(item);
+	if (!SecKeychainItemDelete(item))
+		return EXIT_SUCCESS;
+
+	return EXIT_FAILURE;
 }
 
-static void add_internet_password(void)
+static int add_internet_password(struct credential *c)
 {
 	/* Only store complete credentials */
-	if (!protocol || !host || !username || !password)
-		return;
+	if (!c->protocol || !c->host || !c->username || !c->password)
+		return EXIT_FAILURE;
+
+	if (prepare_internet_password(c))
+		return EXIT_FAILURE;
 
 	if (SecKeychainAddInternetPassword(
-	      KEYCHAIN_ARGS,
-	      KEYCHAIN_ITEM(password),
+	      KEYCHAIN_ARGS(c),
+	      KEYCHAIN_ITEM(c->password),
 	      NULL))
-		return;
-}
+		return EXIT_FAILURE;
 
-static void read_credential(void)
-{
-	char buf[1024];
-
-	while (fgets(buf, sizeof(buf), stdin)) {
-		char *v;
-
-		if (!strcmp(buf, "\n"))
-			break;
-		buf[strlen(buf)-1] = '\0';
-
-		v = strchr(buf, '=');
-		if (!v)
-			die("bad input: %s", buf);
-		*v++ = '\0';
-
-		if (!strcmp(buf, "protocol")) {
-			if (!strcmp(v, "https"))
-				protocol = kSecProtocolTypeHTTPS;
-			else if (!strcmp(v, "http"))
-				protocol = kSecProtocolTypeHTTP;
-			else /* we don't yet handle other protocols */
-				exit(0);
-		}
-		else if (!strcmp(buf, "host")) {
-			char *colon = strchr(v, ':');
-			if (colon) {
-				*colon++ = '\0';
-				port = atoi(colon);
-			}
-			host = xstrdup(v);
-		}
-		else if (!strcmp(buf, "path"))
-			path = xstrdup(v);
-		else if (!strcmp(buf, "username"))
-			username = xstrdup(v);
-		else if (!strcmp(buf, "password"))
-			password = xstrdup(v);
-	}
+	return EXIT_SUCCESS;
 }
 
-int main(int argc, const char **argv)
+/*
+ * Table with helper operation callbacks, used by generic
+ * credential helper main function.
+ */
+struct credential_operation const credential_helper_ops[] =
 {
-	const char *usage =
-		"Usage: git credential-osxkeychain <get|store|erase>";
-
-	if (!argv[1])
-		die(usage);
-
-	read_credential();
-
-	if (!strcmp(argv[1], "get"))
-		find_internet_password();
-	else if (!strcmp(argv[1], "store"))
-		add_internet_password();
-	else if (!strcmp(argv[1], "erase"))
-		delete_internet_password();
-	/* otherwise, ignore unknown action */
-
-	return 0;
-}
+	{ "get",   find_internet_password   },
+	{ "store", add_internet_password    },
+	{ "erase", delete_internet_password },
+	CREDENTIAL_OP_END
+};
-- 
1.7.10.4

Re: [PATCH] contrib: GnomeKeyring support + generic helper implementation

[Junio C Hamano]

"Philipp A. Hartmann" <pah@qo.cx> writes:

> All,
>
> the following patch series proposes enhancements to the credential helper
> implementations in the contrib section.  The detailed development history
> can be found at GitHub [1].
>
>   The first patch adds a GnomeKeyring credential backend.  The GnomeKeyring
> specific parts are based on the work by John Szakmeister, who wrote the
> helper originally for the initial, unpublished version of the credential
> helper protocol.
>
>   The second patch adds a generic implementation of a credential helper
> based on a simplified credential API and common helper functions.  Helpers
> based on this implementation do not need to worry about the credential
> protocol and only need to implement callback functions for the different
> credential operations.
>
>   The third and fourth patches port the existing helpers to this generic
> implementation.
>
> Looking forward to your thoughts.

It struck me somewhat odd to see a new one added as the first step
in the series, and then "the generic", the third patch only to lose
code from the first one, and then use the same code reduction of
existing one with the last one in the series.  A natural progression
would have been the introduction of generic infrastructure
(including the tiny bit this series had to add as part of 4/4) as
the first patch, update existing OSX one to it as the second, and
then build a new Gnome one on the infrastructure as the third and
final patch; that way we have to review less code, too ;-).

I gave it a cursory look; other than getting distracted by
inconsistent coding styles here and there, the patches seemed
reasonably clean and well thought out.

Thanks.

Re: [PATCH] contrib: GnomeKeyring support + generic helper implementation

[Jeff King]

On Fri, Aug 24, 2012 at 11:15:36AM -0700, Junio C Hamano wrote:

> >   The third and fourth patches port the existing helpers to this generic
> > implementation.
> >
> It struck me somewhat odd to see a new one added as the first step
> in the series, and then "the generic", the third patch only to lose
> code from the first one, and then use the same code reduction of
> existing one with the last one in the series.  A natural progression
> would have been the introduction of generic infrastructure
> (including the tiny bit this series had to add as part of 4/4) as
> the first patch, update existing OSX one to it as the second, and
> then build a new Gnome one on the infrastructure as the third and
> final patch; that way we have to review less code, too ;-).

I think this is partially because I talked with Phillipp off-list and
was somewhat unsure how much we wanted to factor out of the helpers. My
initial thought was that the protocol would be sufficiently simple that
helpers would just be stand-alone and not need to share code with each
other. Then the generic bits would not have to worry about being
cross-platform compatible.

However, the shared bits are simple enough that maybe that is not a
concern. An interesting test would be to add a 5/4 porting Erik's win32
credential helper, since that is the platform least like our other ones.

So I am OK with this series, but I am also OK with leaving it at patch
1, and just keeping the implementations separate.

-Peff

Re: [PATCH] contrib: GnomeKeyring support + generic helper implementation

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> However, the shared bits are simple enough that maybe that is not a
> concern. An interesting test would be to add a 5/4 porting Erik's win32
> credential helper, since that is the platform least like our other ones.

Very true.

> So I am OK with this series, but I am also OK with leaving it at patch
> 1, and just keeping the implementations separate.

Amen.

Re: [PATCH] contrib: GnomeKeyring support + generic helper implementation

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> However, the shared bits are simple enough that maybe that is not a
>> concern. An interesting test would be to add a 5/4 porting Erik's win32
>> credential helper, since that is the platform least like our other ones.
>
> Very true.
>
>> So I am OK with this series, but I am also OK with leaving it at patch
>> 1, and just keeping the implementations separate.
>
> Amen.

Just to make sure we do not leave loose ends, could somebody try to
see if the new "generic helper" infrastructure is useful to shrink
Erik's win32 credential helper implementation?

If we see much code reduction and improved clarity, this refactoring
may worth keeping.  Otherwise it may be sufficient to drop the later
ones in the series.  Without knowing which, it is hard to decide.

Thanks.

Re: [PATCH] contrib: GnomeKeyring support + generic helper implementation

[Philipp A. Hartmann]

n 26/08/12 19:46, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Jeff King <peff@peff.net> writes:
>>
>>> However, the shared bits are simple enough that maybe that is not a
>>> concern. An interesting test would be to add a 5/4 porting Erik's win32
>>> credential helper, since that is the platform least like our other ones.
>>
>> Very true.

In principle, I agree that with the current set of helper
implementations, the generic infrastructure may not yet pay off.

>>> So I am OK with this series, but I am also OK with leaving it at patch
>>> 1, and just keeping the implementations separate.
>>
>> Amen.
> 
> Just to make sure we do not leave loose ends, could somebody try to
> see if the new "generic helper" infrastructure is useful to shrink
> Erik's win32 credential helper implementation?

I'll try to give it a shot now, although I won't be able to test it.
I'll send the results as a single 5/4 addition to the previous series.

> If we see much code reduction and improved clarity, this refactoring
> may worth keeping.  Otherwise it may be sufficient to drop the later
> ones in the series.  Without knowing which, it is hard to decide.

If we decide that the generic implementation is useful, I'll then resend
the reordered series adding the generic helper first, then refactoring
the existing ones and adding the new one for GnomeKeyring last.

Thanks,
  Philipp

[PATCH 5/4] wincred: port to generic credential helper (UNTESTED)

[Philipp A. Hartmann]

From: "Philipp A. Hartmann" <pah@qo.cx>

This patch is an experiment to port the wincred helper
to the generic implementation.  As of know, it is
completely untested.

In addition to porting the helper to the generic API,
this patch clears up all passwords from memory, which
reduces the total amount to saved lines.

Signed-off-by: Philipp A. Hartmann <pah@qo.cx>
---

The porting was fairly easy, but due to the lack of a testing
platform, it is completely untested.

Erik: Can you try to have look?

The patch is against the current 'next' with my series applied.
A github repository with all the helpers can also be found at
  git://github.com/pah/git-credential-helper.git

In order to keep things simple, I decided to add the setup of
the binary pipes in the generic helper's main() already.  This
was the only platform-specific change needed in the helper.

Other changes to the generic helper were
 - the addition of an URI field in the credential struct
   (can be reused in the GNOME implementation)
 - the early stop in case of incomplete credentials
   (simplifies handling this case in all three helpers)

The overall code reduction could be increased (also in
the osxkeychain helper), if we add init/cleanup functions
to the generic helper.  I skipped this for now.

Some additions to the wincred code were needed (or at least
motivated) to clean up the memory correctly.  Especially for
the password strings, this seems to be a sane thing to do.
Since the wincred helper duplicates parts of these buffers to
comply with the wchar_t-based Windows API, this required more
manual free's than in the other helpers.

In summary, although the platform and API looked fairly different
from the other platforms, the generic code could be used without
too much pain.

Thanks,
  Philipp

 contrib/credential/helper/credential_helper.c      |   41 ++-
 contrib/credential/helper/credential_helper.h      |    3 +-
 contrib/credential/wincred/.gitignore              |    1 +
 contrib/credential/wincred/Makefile                |   19 +-
 .../credential/wincred/git-credential-wincred.c    |  310 +++++++++-----------
 5 files changed, 199 insertions(+), 175 deletions(-)
 create mode 100644 contrib/credential/wincred/.gitignore

diff --git a/contrib/credential/helper/credential_helper.c b/contrib/credential/helper/credential_helper.c
index e99c2ec..5ddf5a5 100644
--- a/contrib/credential/helper/credential_helper.c
+++ b/contrib/credential/helper/credential_helper.c
@@ -11,6 +11,11 @@
 
 #include <credential_helper.h>
 
+#ifdef WIN32
+#include <fcntl.h>
+#include <io.h>
+#endif
+
 void credential_init(struct credential *c)
 {
 	memset(c, 0, sizeof(*c));
@@ -23,6 +28,7 @@ void credential_clear(struct credential *c)
 	free(c->path);
 	free(c->username);
 	free_password(c->password);
+	free(c->uri);
 
 	credential_init(c);
 }
@@ -79,6 +85,29 @@ int credential_read(struct credential *c)
 		 * learn new lines, and the helpers are updated to match.
 		 */
 	}
+
+	/* Rebuild URI from parts */
+	*buf = '\0';
+	if (c->protocol) {
+		strncat(buf, c->protocol, sizeof(buf));
+		strncat(buf, "://", sizeof(buf));
+	}
+	if (c->username) {
+		strncat(buf, c->username, sizeof(buf));
+		strncat(buf, "@", sizeof(buf));
+	}
+	if (c->host)
+		strncat(buf, c->host, sizeof(buf));
+	if (c->port) {
+		value = buf + strlen(buf);
+		snprintf(value, sizeof(buf)-(value-buf), ":%hd", c->port);
+	}
+	if (c->path) {
+		strncat(buf, "/", sizeof(buf));
+		strncat(buf, c->path, sizeof(buf));
+	}
+	c->uri = xstrdup(buf);
+
 	return 0;
 }
 
@@ -126,6 +155,12 @@ int main(int argc, char *argv[])
 		goto out;
 	}
 
+#ifdef WIN32
+	/* git on Windows uses binary pipes to avoid CRLF-issues */
+	_setmode(_fileno(stdin), _O_BINARY);
+	_setmode(_fileno(stdout), _O_BINARY);
+#endif
+
 	/* lookup operation callback */
 	while(try_op->name && strcmp(argv[1], try_op->name))
 		try_op++;
@@ -138,11 +173,15 @@ int main(int argc, char *argv[])
 	if(ret)
 		goto out;
 
+	if (!cred.protocol || !(cred.host || cred.path)) {
+		ret = EXIT_FAILURE;
+		goto out;
+	}
+
 	/* perform credential operation */
 	ret = (*try_op->op)(&cred);
 
 	credential_write(&cred);
-
 out:
 	credential_clear(&cred);
 	return ret;
diff --git a/contrib/credential/helper/credential_helper.h b/contrib/credential/helper/credential_helper.h
index 76b6e50..822b471 100644
--- a/contrib/credential/helper/credential_helper.h
+++ b/contrib/credential/helper/credential_helper.h
@@ -25,10 +25,11 @@ struct credential
 	char          *path;
 	char          *username;
 	char          *password;
+	char          *uri;     /* <protocol>://[username@][host[:port]][/path] */
 };
 
 #define CREDENTIAL_INIT \
-  { NULL,NULL,0,NULL,NULL,NULL }
+  { NULL,NULL,0,NULL,NULL,NULL,NULL }
 
 void credential_init(struct credential *c);
 void credential_clear(struct credential *c);
diff --git a/contrib/credential/wincred/.gitignore b/contrib/credential/wincred/.gitignore
new file mode 100644
index 0000000..4780e4e
--- /dev/null
+++ b/contrib/credential/wincred/.gitignore
@@ -0,0 +1 @@
+git-credential-wincred.exe
diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile
index bad45ca..ee7a8ef 100644
--- a/contrib/credential/wincred/Makefile
+++ b/contrib/credential/wincred/Makefile
@@ -1,4 +1,5 @@
-all: git-credential-wincred.exe
+MAIN:=git-credential-wincred
+all:: $(MAIN)
 
 CC = gcc
 RM = rm -f
@@ -7,8 +8,18 @@ CFLAGS = -O2 -Wall
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
-git-credential-wincred.exe : git-credential-wincred.c
-	$(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@
+HELPER:=../helper
+VPATH +=$(HELPER)
+
+SRCS:=$(MAIN).c
+SRCS+=credential_helper.c
+OBJS:=$(SRCS:.c=.o)
+
+%.o: %.c
+	$(CC) $(CFLAGS) $(CPPFLAGS) -I$(HELPER) -o $@ -c $<
+
+$(MAIN).exe: $(OBJS)
+	$(LINK.o) $^ $(LOADLIBES) $(LDLIBS) -o $@
 
 clean:
-	$(RM) git-credential-wincred.exe
+	$(RM) $(MAIN).exe $(OBJS)
diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c
index cbaec5f..721e59f 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -7,37 +7,6 @@
 #include <io.h>
 #include <fcntl.h>
 
-/* common helpers */
-
-static void die(const char *err, ...)
-{
-	char msg[4096];
-	va_list params;
-	va_start(params, err);
-	vsnprintf(msg, sizeof(msg), err, params);
-	fprintf(stderr, "%s\n", msg);
-	va_end(params);
-	exit(1);
-}
-
-static void *xmalloc(size_t size)
-{
-	void *ret = malloc(size);
-	if (!ret && !size)
-		ret = malloc(1);
-	if (!ret)
-		 die("Out of memory");
-	return ret;
-}
-
-static char *xstrdup(const char *str)
-{
-	char *ret = strdup(str);
-	if (!ret)
-		die("Out of memory");
-	return ret;
-}
-
 /* MinGW doesn't have wincred.h, so we need to define stuff */
 
 typedef struct _CREDENTIAL_ATTRIBUTEW {
@@ -84,13 +53,16 @@ static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW;
 static CredFreeT CredFree;
 static CredDeleteWT CredDeleteW;
 
-static void load_cred_funcs(void)
+
+static int load_cred_funcs(void)
 {
 	/* load DLLs */
 	advapi = LoadLibrary("advapi32.dll");
 	credui = LoadLibrary("credui.dll");
-	if (!advapi || !credui)
-		die("failed to load DLLs");
+	if (!advapi || !credui) {
+		error("failed to load DLLs");
+		return EXIT_FAILURE;
+	}
 
 	/* get function pointers */
 	CredWriteW = (CredWriteWT)GetProcAddress(advapi, "CredWriteW");
@@ -104,28 +76,58 @@ static void load_cred_funcs(void)
 	CredDeleteW = (CredDeleteWT)GetProcAddress(advapi, "CredDeleteW");
 	if (!CredWriteW || !CredUnPackAuthenticationBufferW ||
 	    !CredEnumerateW || !CredPackAuthenticationBufferW || !CredFree ||
-	    !CredDeleteW)
-		die("failed to load functions");
+	    !CredDeleteW) {
+		error("failed to load functions");
+		return EXIT_FAILURE;
+	}
+	return EXIT_SUCCESS;
 }
 
 static char target_buf[1024];
-static char *protocol, *host, *path, *username;
-static WCHAR *wusername, *password, *target;
+static char port_buf[8];
+static WCHAR *wusername, *wpassword;
 
-static void write_item(const char *what, WCHAR *wbuf)
+static WCHAR *utf8_to_utf16_dup(const char *str)
 {
-	char *buf;
-	int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, NULL, 0, NULL,
-	    FALSE);
-	buf = xmalloc(len);
-
-	if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, buf, len, NULL, FALSE))
-		die("WideCharToMultiByte failed!");
-
-	printf("%s=", what);
-	fwrite(buf, 1, len - 1, stdout);
-	putchar('\n');
-	free(buf);
+	int wlen = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0);
+	WCHAR *wstr = xmalloc(sizeof(WCHAR) * wlen);
+	if (!MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, wlen))
+		die("MultiByteToWideChar failed");
+	return wstr;
+}
+
+static char *utf16_to_utf8_dup(const WCHAR *wstr)
+{
+	int len = WideCharToMultiByte(CP_UTF8, 0, wstr, -1, NULL, 0, NULL,
+		FALSE);
+	char *str = xmalloc(len);
+	if (!WideCharToMultiByte(CP_UTF8, 0, wstr, -1, str, len, NULL, FALSE))
+		die("WideCharToMultiByte failed");
+	return str;
+}
+
+static void free_wpassword(WCHAR *pass)
+{
+	WCHAR *w = pass;
+	if(!pass)
+		return;
+	while(*w) *w++ = L'\0';
+	free(pass);
+}
+
+static int prepare_credential(struct credential *c)
+{
+	if (load_cred_funcs() )
+		return EXIT_FAILURE;
+
+	if (c->username)
+		wusername = utf8_to_utf16_dup(c->username);
+	if (c->password)
+		wpassword = utf8_to_utf16_dup(c->password);
+	if (c->port) {
+		snprintf(port_buf,"%hd",c->port);
+	}
+	return EXIT_SUCCESS;
 }
 
 static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword,
@@ -143,24 +145,28 @@ static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword,
 	return 0; /* not found */
 }
 
-static int match_cred(const CREDENTIALW *cred)
+static int match_cred(const CREDENTIALW *cred, const struct credential *c)
 {
 	return (!wusername || !wcscmp(wusername, cred->UserName)) &&
-	    match_attr(cred, L"git_protocol", protocol) &&
-	    match_attr(cred, L"git_host", host) &&
-	    match_attr(cred, L"git_path", path);
+	    match_attr(cred, L"git_protocol", c->protocol) &&
+	    match_attr(cred, L"git_host", c->host) &&
+	    (c->port && match_attr(cred, L"git_port", port_buf)) &&
+	    match_attr(cred, L"git_path", c->path);
 }
 
-static void get_credential(void)
+static int get_credential(struct credential *c)
 {
-	WCHAR *user_buf, *pass_buf;
+	WCHAR *user_buf = NULL, *pass_buf = NULL;
 	DWORD user_buf_size = 0, pass_buf_size = 0;
 	CREDENTIALW **creds, *cred = NULL;
 	DWORD num_creds;
-	int i;
+	int i, ret = EXIT_SUCCESS;
+
+	if (prepare_credential(c))
+		return EXIT_FAILURE;
 
 	if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds))
-		return;
+		return EXIT_FAILURE;
 
 	/* search for the first credential that matches username */
 	for (i = 0; i < num_creds; ++i)
@@ -169,7 +175,7 @@ static void get_credential(void)
 			break;
 		}
 	if (!cred)
-		return;
+		goto out;
 
 	CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
 	    cred->CredentialBlobSize, NULL, &user_buf_size, NULL, NULL,
@@ -180,20 +186,29 @@ static void get_credential(void)
 
 	if (!CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
 	    cred->CredentialBlobSize, user_buf, &user_buf_size, NULL, NULL,
-	    pass_buf, &pass_buf_size))
-		die("CredUnPackAuthenticationBuffer failed");
-
-	CredFree(creds);
+	    pass_buf, &pass_buf_size)) {
+		error("CredUnPackAuthenticationBuffer failed");
+		ret = EXIT_FAILURE;
+		goto out;
+	}
 
 	/* zero-terminate (sizes include zero-termination) */
 	user_buf[user_buf_size - 1] = L'\0';
 	pass_buf[pass_buf_size - 1] = L'\0';
 
-	write_item("username", user_buf);
-	write_item("password", pass_buf);
+	if (!c->username)
+		c->username = utf16_to_utf8_dup(user_buf);
+	free_password(c->password);
+	c->password = utf16_to_utf8_dup(pass_buf);
+
+out:
+	CredFree(creds);
 
 	free(user_buf);
-	free(pass_buf);
+	free(wusername);
+	free_wpassword(pass_buf);
+
+	return ret;
 }
 
 static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword,
@@ -205,29 +220,42 @@ static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword,
 	attr->Value = (LPBYTE)value;
 }
 
-static void store_credential(void)
+static int store_credential(struct credential *c)
 {
 	CREDENTIALW cred;
 	BYTE *auth_buf;
 	DWORD auth_buf_size = 0;
 	CREDENTIAL_ATTRIBUTEW attrs[CRED_MAX_ATTRIBUTES];
+	WCHAR *wtarget;
+	int ret = EXIT_SUCCESS;
 
-	if (!wusername || !password)
-		return;
+	if (prepare_credential(c))
+		return EXIT_FAILURE;
+
+	/* prepare 'target', the unique key for the credential */
+	strncat(target_buf,"git:",sizeof(target_buf));
+	strncat(target_buf,c->uri,sizeof(target_buf));
+	wtarget = utf8_to_utf16_dup(target_buf);
+
+	if (!wusername || !wpassword)
+		return EXIT_FAILURE;
 
 	/* query buffer size */
-	CredPackAuthenticationBufferW(0, wusername, password,
+	CredPackAuthenticationBufferW(0, wusername, wpassword,
 	    NULL, &auth_buf_size);
 
 	auth_buf = xmalloc(auth_buf_size);
 
-	if (!CredPackAuthenticationBufferW(0, wusername, password,
-	    auth_buf, &auth_buf_size))
-		die("CredPackAuthenticationBuffer failed");
+	if (!CredPackAuthenticationBufferW(0, wusername, wpassword,
+	    auth_buf, &auth_buf_size)) {
+		error("CredPackAuthenticationBuffer failed");
+		ret = EXIT_FAILURE;
+		goto out;
+	}
 
 	cred.Flags = 0;
 	cred.Type = CRED_TYPE_GENERIC;
-	cred.TargetName = target;
+	cred.TargetName = wtarget;
 	cred.Comment = L"saved by git-credential-wincred";
 	cred.CredentialBlobSize = auth_buf_size;
 	cred.CredentialBlob = auth_buf;
@@ -237,121 +265,65 @@ static void store_credential(void)
 	cred.TargetAlias = NULL;
 	cred.UserName = wusername;
 
-	write_attr(attrs, L"git_protocol", protocol);
+	write_attr(attrs, L"git_protocol", c->protocol);
 
-	if (host) {
-		write_attr(attrs + cred.AttributeCount, L"git_host", host);
+	if (c->host) {
+		write_attr(attrs + cred.AttributeCount, L"git_host", c->host);
 		cred.AttributeCount++;
 	}
 
-	if (path) {
-		write_attr(attrs + cred.AttributeCount, L"git_path", path);
+	if (c->port) {
+		write_attr(attrs + cred.AttributeCount, L"git_port", port_buf);
 		cred.AttributeCount++;
 	}
 
-	if (!CredWriteW(&cred, 0))
-		die("CredWrite failed");
+	if (c->path) {
+		write_attr(attrs + cred.AttributeCount, L"git_path", c->path);
+		cred.AttributeCount++;
+	}
+
+	if (!CredWriteW(&cred, 0)) {
+		error("CredWrite failed");
+		ret = EXIT_FAILURE;
+	}
+
+out:
+	free(auth_buf);
+	free(wusername);
+	free_wpassword(wpassword);
+	free(wtarget);
+	return ret;
 }
 
-static void erase_credential(void)
+static int erase_credential(struct credential *c)
 {
 	CREDENTIALW **creds;
 	DWORD num_creds;
 	int i;
 
+	if (prepare_credential(c))
+		return EXIT_FAILURE;
+
 	if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds))
-		return;
+		return EXIT_FAILURE;
 
 	for (i = 0; i < num_creds; ++i) {
-		if (match_cred(creds[i]))
+		if (match_cred(creds[i],c))
 			CredDeleteW(creds[i]->TargetName, creds[i]->Type, 0);
 	}
 
 	CredFree(creds);
+	return EXIT_SUCCESS;
 }
 
-static WCHAR *utf8_to_utf16_dup(const char *str)
-{
-	int wlen = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0);
-	WCHAR *wstr = xmalloc(sizeof(WCHAR) * wlen);
-	MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, wlen);
-	return wstr;
-}
-
-static void read_credential(void)
-{
-	char buf[1024];
-
-	while (fgets(buf, sizeof(buf), stdin)) {
-		char *v;
-
-		if (!strcmp(buf, "\n"))
-			break;
-		buf[strlen(buf)-1] = '\0';
-
-		v = strchr(buf, '=');
-		if (!v)
-			die("bad input: %s", buf);
-		*v++ = '\0';
-
-		if (!strcmp(buf, "protocol"))
-			protocol = xstrdup(v);
-		else if (!strcmp(buf, "host"))
-			host = xstrdup(v);
-		else if (!strcmp(buf, "path"))
-			path = xstrdup(v);
-		else if (!strcmp(buf, "username")) {
-			username = xstrdup(v);
-			wusername = utf8_to_utf16_dup(v);
-		} else if (!strcmp(buf, "password"))
-			password = utf8_to_utf16_dup(v);
-		else
-			die("unrecognized input");
-	}
-}
-
-int main(int argc, char *argv[])
+/*
+ * Table with helper operation callbacks, used by generic
+ * credential helper main function.
+ */
+struct credential_operation const credential_helper_ops[] =
 {
-	const char *usage =
-	    "Usage: git credential-wincred <get|store|erase>\n";
-
-	if (!argv[1])
-		die(usage);
-
-	/* git use binary pipes to avoid CRLF-issues */
-	_setmode(_fileno(stdin), _O_BINARY);
-	_setmode(_fileno(stdout), _O_BINARY);
-
-	read_credential();
-
-	load_cred_funcs();
-
-	if (!protocol || !(host || path))
-		return 0;
-
-	/* prepare 'target', the unique key for the credential */
-	strncat(target_buf, "git:", sizeof(target_buf));
-	strncat(target_buf, protocol, sizeof(target_buf));
-	strncat(target_buf, "://", sizeof(target_buf));
-	if (username) {
-		strncat(target_buf, username, sizeof(target_buf));
-		strncat(target_buf, "@", sizeof(target_buf));
-	}
-	if (host)
-		strncat(target_buf, host, sizeof(target_buf));
-	if (path) {
-		strncat(target_buf, "/", sizeof(target_buf));
-		strncat(target_buf, path, sizeof(target_buf));
-	}
-
-	target = utf8_to_utf16_dup(target_buf);
-
-	if (!strcmp(argv[1], "get"))
-		get_credential();
-	else if (!strcmp(argv[1], "store"))
-		store_credential();
-	else if (!strcmp(argv[1], "erase"))
-		erase_credential();
-	/* otherwise, ignore unknown action */
-	return 0;
-}
+	{ "get",   get_credential   },
+	{ "store", store_credential },
+	{ "erase", erase_credential },
+	CREDENTIAL_OP_END
+};
-- 
1.7.10.4

[PATCH 5/4 v2] wincred: port to generic credential helper (UNTESTED)

[Philipp A. Hartmann]

From: "Philipp A. Hartmann" <pah@qo.cx>

This patch is an experiment to port the wincred helper
to the generic implementation.  As of know, it is
completely untested.

In addition to porting the helper to the generic API,
this patch clears up all passwords from memory, which
reduces the total amount to saved lines.

This version adds the missing xmalloc() implementation.

Signed-off-by: Philipp A. Hartmann <pah@qo.cx>
---

The previous version of the patch somehow lacked the xmalloc
implementation moved to credential_helper.h.  Sorry for the
inconvenience.

Apart from being still untested, this version at least matches
my tree at git://github.com/pah/git-credential-helper.git.

 contrib/credential/helper/credential_helper.c      |   41 ++-
 contrib/credential/helper/credential_helper.h      |   14 +-
 contrib/credential/wincred/.gitignore              |    1 +
 contrib/credential/wincred/Makefile                |   19 +-
 .../credential/wincred/git-credential-wincred.c    |  310 +++++++++-----------
 5 files changed, 210 insertions(+), 175 deletions(-)
 create mode 100644 contrib/credential/wincred/.gitignore

diff --git a/contrib/credential/helper/credential_helper.c b/contrib/credential/helper/credential_helper.c
index e99c2ec..5ddf5a5 100644
--- a/contrib/credential/helper/credential_helper.c
+++ b/contrib/credential/helper/credential_helper.c
@@ -11,6 +11,11 @@
 
 #include <credential_helper.h>
 
+#ifdef WIN32
+#include <fcntl.h>
+#include <io.h>
+#endif
+
 void credential_init(struct credential *c)
 {
 	memset(c, 0, sizeof(*c));
@@ -23,6 +28,7 @@ void credential_clear(struct credential *c)
 	free(c->path);
 	free(c->username);
 	free_password(c->password);
+	free(c->uri);
 
 	credential_init(c);
 }
@@ -79,6 +85,29 @@ int credential_read(struct credential *c)
 		 * learn new lines, and the helpers are updated to match.
 		 */
 	}
+
+	/* Rebuild URI from parts */
+	*buf = '\0';
+	if (c->protocol) {
+		strncat(buf, c->protocol, sizeof(buf));
+		strncat(buf, "://", sizeof(buf));
+	}
+	if (c->username) {
+		strncat(buf, c->username, sizeof(buf));
+		strncat(buf, "@", sizeof(buf));
+	}
+	if (c->host)
+		strncat(buf, c->host, sizeof(buf));
+	if (c->port) {
+		value = buf + strlen(buf);
+		snprintf(value, sizeof(buf)-(value-buf), ":%hd", c->port);
+	}
+	if (c->path) {
+		strncat(buf, "/", sizeof(buf));
+		strncat(buf, c->path, sizeof(buf));
+	}
+	c->uri = xstrdup(buf);
+
 	return 0;
 }
 
@@ -126,6 +155,12 @@ int main(int argc, char *argv[])
 		goto out;
 	}
 
+#ifdef WIN32
+	/* git on Windows uses binary pipes to avoid CRLF-issues */
+	_setmode(_fileno(stdin), _O_BINARY);
+	_setmode(_fileno(stdout), _O_BINARY);
+#endif
+
 	/* lookup operation callback */
 	while(try_op->name && strcmp(argv[1], try_op->name))
 		try_op++;
@@ -138,11 +173,15 @@ int main(int argc, char *argv[])
 	if(ret)
 		goto out;
 
+	if (!cred.protocol || !(cred.host || cred.path)) {
+		ret = EXIT_FAILURE;
+		goto out;
+	}
+
 	/* perform credential operation */
 	ret = (*try_op->op)(&cred);
 
 	credential_write(&cred);
-
 out:
 	credential_clear(&cred);
 	return ret;
diff --git a/contrib/credential/helper/credential_helper.h b/contrib/credential/helper/credential_helper.h
index 76b6e50..7e73fc6 100644
--- a/contrib/credential/helper/credential_helper.h
+++ b/contrib/credential/helper/credential_helper.h
@@ -25,10 +25,11 @@ struct credential
 	char          *path;
 	char          *username;
 	char          *password;
+	char          *uri;     /* <protocol>://[username@][host[:port]][/path] */
 };
 
 #define CREDENTIAL_INIT \
-  { NULL,NULL,0,NULL,NULL,NULL }
+  { NULL,NULL,0,NULL,NULL,NULL,NULL }
 
 void credential_init(struct credential *c);
 void credential_clear(struct credential *c);
@@ -104,6 +105,17 @@ static inline void die_errno(int err)
 	exit(EXIT_FAILURE);
 }
 
+static inline void *xmalloc(size_t size)
+{
+  void *ret = malloc(size);
+	if (!ret && !size)
+		ret = malloc(1);
+	if (!ret)
+		 die_errno(errno);
+
+	return ret;
+}
+
 static inline char *xstrdup(const char *str)
 {
 	char *ret = strdup(str);
diff --git a/contrib/credential/wincred/.gitignore b/contrib/credential/wincred/.gitignore
new file mode 100644
index 0000000..4780e4e
--- /dev/null
+++ b/contrib/credential/wincred/.gitignore
@@ -0,0 +1 @@
+git-credential-wincred.exe
diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile
index bad45ca..ee7a8ef 100644
--- a/contrib/credential/wincred/Makefile
+++ b/contrib/credential/wincred/Makefile
@@ -1,4 +1,5 @@
-all: git-credential-wincred.exe
+MAIN:=git-credential-wincred
+all:: $(MAIN)
 
 CC = gcc
 RM = rm -f
@@ -7,8 +8,18 @@ CFLAGS = -O2 -Wall
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
-git-credential-wincred.exe : git-credential-wincred.c
-	$(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@
+HELPER:=../helper
+VPATH +=$(HELPER)
+
+SRCS:=$(MAIN).c
+SRCS+=credential_helper.c
+OBJS:=$(SRCS:.c=.o)
+
+%.o: %.c
+	$(CC) $(CFLAGS) $(CPPFLAGS) -I$(HELPER) -o $@ -c $<
+
+$(MAIN).exe: $(OBJS)
+	$(LINK.o) $^ $(LOADLIBES) $(LDLIBS) -o $@
 
 clean:
-	$(RM) git-credential-wincred.exe
+	$(RM) $(MAIN).exe $(OBJS)
diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c
index cbaec5f..721e59f 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -7,37 +7,6 @@
 #include <io.h>
 #include <fcntl.h>
 
-/* common helpers */
-
-static void die(const char *err, ...)
-{
-	char msg[4096];
-	va_list params;
-	va_start(params, err);
-	vsnprintf(msg, sizeof(msg), err, params);
-	fprintf(stderr, "%s\n", msg);
-	va_end(params);
-	exit(1);
-}
-
-static void *xmalloc(size_t size)
-{
-	void *ret = malloc(size);
-	if (!ret && !size)
-		ret = malloc(1);
-	if (!ret)
-		 die("Out of memory");
-	return ret;
-}
-
-static char *xstrdup(const char *str)
-{
-	char *ret = strdup(str);
-	if (!ret)
-		die("Out of memory");
-	return ret;
-}
-
 /* MinGW doesn't have wincred.h, so we need to define stuff */
 
 typedef struct _CREDENTIAL_ATTRIBUTEW {
@@ -84,13 +53,16 @@ static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW;
 static CredFreeT CredFree;
 static CredDeleteWT CredDeleteW;
 
-static void load_cred_funcs(void)
+
+static int load_cred_funcs(void)
 {
 	/* load DLLs */
 	advapi = LoadLibrary("advapi32.dll");
 	credui = LoadLibrary("credui.dll");
-	if (!advapi || !credui)
-		die("failed to load DLLs");
+	if (!advapi || !credui) {
+		error("failed to load DLLs");
+		return EXIT_FAILURE;
+	}
 
 	/* get function pointers */
 	CredWriteW = (CredWriteWT)GetProcAddress(advapi, "CredWriteW");
@@ -104,28 +76,58 @@ static void load_cred_funcs(void)
 	CredDeleteW = (CredDeleteWT)GetProcAddress(advapi, "CredDeleteW");
 	if (!CredWriteW || !CredUnPackAuthenticationBufferW ||
 	    !CredEnumerateW || !CredPackAuthenticationBufferW || !CredFree ||
-	    !CredDeleteW)
-		die("failed to load functions");
+	    !CredDeleteW) {
+		error("failed to load functions");
+		return EXIT_FAILURE;
+	}
+	return EXIT_SUCCESS;
 }
 
 static char target_buf[1024];
-static char *protocol, *host, *path, *username;
-static WCHAR *wusername, *password, *target;
+static char port_buf[8];
+static WCHAR *wusername, *wpassword;
 
-static void write_item(const char *what, WCHAR *wbuf)
+static WCHAR *utf8_to_utf16_dup(const char *str)
 {
-	char *buf;
-	int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, NULL, 0, NULL,
-	    FALSE);
-	buf = xmalloc(len);
-
-	if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, buf, len, NULL, FALSE))
-		die("WideCharToMultiByte failed!");
-
-	printf("%s=", what);
-	fwrite(buf, 1, len - 1, stdout);
-	putchar('\n');
-	free(buf);
+	int wlen = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0);
+	WCHAR *wstr = xmalloc(sizeof(WCHAR) * wlen);
+	if (!MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, wlen))
+		die("MultiByteToWideChar failed");
+	return wstr;
+}
+
+static char *utf16_to_utf8_dup(const WCHAR *wstr)
+{
+	int len = WideCharToMultiByte(CP_UTF8, 0, wstr, -1, NULL, 0, NULL,
+		FALSE);
+	char *str = xmalloc(len);
+	if (!WideCharToMultiByte(CP_UTF8, 0, wstr, -1, str, len, NULL, FALSE))
+		die("WideCharToMultiByte failed");
+	return str;
+}
+
+static void free_wpassword(WCHAR *pass)
+{
+	WCHAR *w = pass;
+	if(!pass)
+		return;
+	while(*w) *w++ = L'\0';
+	free(pass);
+}
+
+static int prepare_credential(struct credential *c)
+{
+	if (load_cred_funcs() )
+		return EXIT_FAILURE;
+
+	if (c->username)
+		wusername = utf8_to_utf16_dup(c->username);
+	if (c->password)
+		wpassword = utf8_to_utf16_dup(c->password);
+	if (c->port) {
+		snprintf(port_buf,"%hd",c->port);
+	}
+	return EXIT_SUCCESS;
 }
 
 static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword,
@@ -143,24 +145,28 @@ static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword,
 	return 0; /* not found */
 }
 
-static int match_cred(const CREDENTIALW *cred)
+static int match_cred(const CREDENTIALW *cred, const struct credential *c)
 {
 	return (!wusername || !wcscmp(wusername, cred->UserName)) &&
-	    match_attr(cred, L"git_protocol", protocol) &&
-	    match_attr(cred, L"git_host", host) &&
-	    match_attr(cred, L"git_path", path);
+	    match_attr(cred, L"git_protocol", c->protocol) &&
+	    match_attr(cred, L"git_host", c->host) &&
+	    (c->port && match_attr(cred, L"git_port", port_buf)) &&
+	    match_attr(cred, L"git_path", c->path);
 }
 
-static void get_credential(void)
+static int get_credential(struct credential *c)
 {
-	WCHAR *user_buf, *pass_buf;
+	WCHAR *user_buf = NULL, *pass_buf = NULL;
 	DWORD user_buf_size = 0, pass_buf_size = 0;
 	CREDENTIALW **creds, *cred = NULL;
 	DWORD num_creds;
-	int i;
+	int i, ret = EXIT_SUCCESS;
+
+	if (prepare_credential(c))
+		return EXIT_FAILURE;
 
 	if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds))
-		return;
+		return EXIT_FAILURE;
 
 	/* search for the first credential that matches username */
 	for (i = 0; i < num_creds; ++i)
@@ -169,7 +175,7 @@ static void get_credential(void)
 			break;
 		}
 	if (!cred)
-		return;
+		goto out;
 
 	CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
 	    cred->CredentialBlobSize, NULL, &user_buf_size, NULL, NULL,
@@ -180,20 +186,29 @@ static void get_credential(void)
 
 	if (!CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
 	    cred->CredentialBlobSize, user_buf, &user_buf_size, NULL, NULL,
-	    pass_buf, &pass_buf_size))
-		die("CredUnPackAuthenticationBuffer failed");
-
-	CredFree(creds);
+	    pass_buf, &pass_buf_size)) {
+		error("CredUnPackAuthenticationBuffer failed");
+		ret = EXIT_FAILURE;
+		goto out;
+	}
 
 	/* zero-terminate (sizes include zero-termination) */
 	user_buf[user_buf_size - 1] = L'\0';
 	pass_buf[pass_buf_size - 1] = L'\0';
 
-	write_item("username", user_buf);
-	write_item("password", pass_buf);
+	if (!c->username)
+		c->username = utf16_to_utf8_dup(user_buf);
+	free_password(c->password);
+	c->password = utf16_to_utf8_dup(pass_buf);
+
+out:
+	CredFree(creds);
 
 	free(user_buf);
-	free(pass_buf);
+	free(wusername);
+	free_wpassword(pass_buf);
+
+	return ret;
 }
 
 static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword,
@@ -205,29 +220,42 @@ static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword,
 	attr->Value = (LPBYTE)value;
 }
 
-static void store_credential(void)
+static int store_credential(struct credential *c)
 {
 	CREDENTIALW cred;
 	BYTE *auth_buf;
 	DWORD auth_buf_size = 0;
 	CREDENTIAL_ATTRIBUTEW attrs[CRED_MAX_ATTRIBUTES];
+	WCHAR *wtarget;
+	int ret = EXIT_SUCCESS;
 
-	if (!wusername || !password)
-		return;
+	if (prepare_credential(c))
+		return EXIT_FAILURE;
+
+	/* prepare 'target', the unique key for the credential */
+	strncat(target_buf,"git:",sizeof(target_buf));
+	strncat(target_buf,c->uri,sizeof(target_buf));
+	wtarget = utf8_to_utf16_dup(target_buf);
+
+	if (!wusername || !wpassword)
+		return EXIT_FAILURE;
 
 	/* query buffer size */
-	CredPackAuthenticationBufferW(0, wusername, password,
+	CredPackAuthenticationBufferW(0, wusername, wpassword,
 	    NULL, &auth_buf_size);
 
 	auth_buf = xmalloc(auth_buf_size);
 
-	if (!CredPackAuthenticationBufferW(0, wusername, password,
-	    auth_buf, &auth_buf_size))
-		die("CredPackAuthenticationBuffer failed");
+	if (!CredPackAuthenticationBufferW(0, wusername, wpassword,
+	    auth_buf, &auth_buf_size)) {
+		error("CredPackAuthenticationBuffer failed");
+		ret = EXIT_FAILURE;
+		goto out;
+	}
 
 	cred.Flags = 0;
 	cred.Type = CRED_TYPE_GENERIC;
-	cred.TargetName = target;
+	cred.TargetName = wtarget;
 	cred.Comment = L"saved by git-credential-wincred";
 	cred.CredentialBlobSize = auth_buf_size;
 	cred.CredentialBlob = auth_buf;
@@ -237,121 +265,65 @@ static void store_credential(void)
 	cred.TargetAlias = NULL;
 	cred.UserName = wusername;
 
-	write_attr(attrs, L"git_protocol", protocol);
+	write_attr(attrs, L"git_protocol", c->protocol);
 
-	if (host) {
-		write_attr(attrs + cred.AttributeCount, L"git_host", host);
+	if (c->host) {
+		write_attr(attrs + cred.AttributeCount, L"git_host", c->host);
 		cred.AttributeCount++;
 	}
 
-	if (path) {
-		write_attr(attrs + cred.AttributeCount, L"git_path", path);
+	if (c->port) {
+		write_attr(attrs + cred.AttributeCount, L"git_port", port_buf);
 		cred.AttributeCount++;
 	}
 
-	if (!CredWriteW(&cred, 0))
-		die("CredWrite failed");
+	if (c->path) {
+		write_attr(attrs + cred.AttributeCount, L"git_path", c->path);
+		cred.AttributeCount++;
+	}
+
+	if (!CredWriteW(&cred, 0)) {
+		error("CredWrite failed");
+		ret = EXIT_FAILURE;
+	}
+
+out:
+	free(auth_buf);
+	free(wusername);
+	free_wpassword(wpassword);
+	free(wtarget);
+	return ret;
 }
 
-static void erase_credential(void)
+static int erase_credential(struct credential *c)
 {
 	CREDENTIALW **creds;
 	DWORD num_creds;
 	int i;
 
+	if (prepare_credential(c))
+		return EXIT_FAILURE;
+
 	if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds))
-		return;
+		return EXIT_FAILURE;
 
 	for (i = 0; i < num_creds; ++i) {
-		if (match_cred(creds[i]))
+		if (match_cred(creds[i],c))
 			CredDeleteW(creds[i]->TargetName, creds[i]->Type, 0);
 	}
 
 	CredFree(creds);
+	return EXIT_SUCCESS;
 }
 
-static WCHAR *utf8_to_utf16_dup(const char *str)
-{
-	int wlen = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0);
-	WCHAR *wstr = xmalloc(sizeof(WCHAR) * wlen);
-	MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, wlen);
-	return wstr;
-}
-
-static void read_credential(void)
-{
-	char buf[1024];
-
-	while (fgets(buf, sizeof(buf), stdin)) {
-		char *v;
-
-		if (!strcmp(buf, "\n"))
-			break;
-		buf[strlen(buf)-1] = '\0';
-
-		v = strchr(buf, '=');
-		if (!v)
-			die("bad input: %s", buf);
-		*v++ = '\0';
-
-		if (!strcmp(buf, "protocol"))
-			protocol = xstrdup(v);
-		else if (!strcmp(buf, "host"))
-			host = xstrdup(v);
-		else if (!strcmp(buf, "path"))
-			path = xstrdup(v);
-		else if (!strcmp(buf, "username")) {
-			username = xstrdup(v);
-			wusername = utf8_to_utf16_dup(v);
-		} else if (!strcmp(buf, "password"))
-			password = utf8_to_utf16_dup(v);
-		else
-			die("unrecognized input");
-	}
-}
-
-int main(int argc, char *argv[])
+/*
+ * Table with helper operation callbacks, used by generic
+ * credential helper main function.
+ */
+struct credential_operation const credential_helper_ops[] =
 {
-	const char *usage =
-	    "Usage: git credential-wincred <get|store|erase>\n";
-
-	if (!argv[1])
-		die(usage);
-
-	/* git use binary pipes to avoid CRLF-issues */
-	_setmode(_fileno(stdin), _O_BINARY);
-	_setmode(_fileno(stdout), _O_BINARY);
-
-	read_credential();
-
-	load_cred_funcs();
-
-	if (!protocol || !(host || path))
-		return 0;
-
-	/* prepare 'target', the unique key for the credential */
-	strncat(target_buf, "git:", sizeof(target_buf));
-	strncat(target_buf, protocol, sizeof(target_buf));
-	strncat(target_buf, "://", sizeof(target_buf));
-	if (username) {
-		strncat(target_buf, username, sizeof(target_buf));
-		strncat(target_buf, "@", sizeof(target_buf));
-	}
-	if (host)
-		strncat(target_buf, host, sizeof(target_buf));
-	if (path) {
-		strncat(target_buf, "/", sizeof(target_buf));
-		strncat(target_buf, path, sizeof(target_buf));
-	}
-
-	target = utf8_to_utf16_dup(target_buf);
-
-	if (!strcmp(argv[1], "get"))
-		get_credential();
-	else if (!strcmp(argv[1], "store"))
-		store_credential();
-	else if (!strcmp(argv[1], "erase"))
-		erase_credential();
-	/* otherwise, ignore unknown action */
-	return 0;
-}
+	{ "get",   get_credential   },
+	{ "store", store_credential },
+	{ "erase", erase_credential },
+	CREDENTIAL_OP_END
+};
-- 
1.7.10.4

Re: [PATCH 5/4] wincred: port to generic credential helper (UNTESTED)

[Erik Faye-Lund]

On Mon, Aug 27, 2012 at 12:04 AM, Philipp A. Hartmann <pah@qo.cx> wrote:
> From: "Philipp A. Hartmann" <pah@qo.cx>
>
> This patch is an experiment to port the wincred helper
> to the generic implementation.  As of know, it is
> completely untested.
>
> In addition to porting the helper to the generic API,
> this patch clears up all passwords from memory, which
> reduces the total amount to saved lines.
>
> Signed-off-by: Philipp A. Hartmann <pah@qo.cx>
> ---
>
> The porting was fairly easy, but due to the lack of a testing
> platform, it is completely untested.
>
> Erik: Can you try to have look?

Sorry for the late reply, I'm currently in bed with pneumonia.

But I gave it a quick go, but as-is it's a NACK; a wall of warnings and errors.

But with this patch on top, it seems to at least compile OK:

---8<---
diff --git a/contrib/credential/helper/credential_helper.h
b/contrib/credential/helper/credential_helper.h
index 7e73fc6..13b611e 100644
--- a/contrib/credential/helper/credential_helper.h
+++ b/contrib/credential/helper/credential_helper.h
@@ -125,6 +125,7 @@ static inline char *xstrdup(const char *str)
 	return ret;
 }

+#ifndef NO_STRNDUP
 static inline char *xstrndup(const char *str, size_t len)
 {
 	char *ret = strndup(str,len);
@@ -133,5 +134,6 @@ static inline char *xstrndup(const char *str, size_t len)

 	return ret;
 }
+#endif

 #endif /* CREDENTIAL_HELPER_H_INCLUDED_ */
diff --git a/contrib/credential/wincred/Makefile
b/contrib/credential/wincred/Makefile
index ee7a8ef..3900322 100644
--- a/contrib/credential/wincred/Makefile
+++ b/contrib/credential/wincred/Makefile
@@ -1,9 +1,10 @@
 MAIN:=git-credential-wincred
-all:: $(MAIN)
+all:: $(MAIN).exe

 CC = gcc
 RM = rm -f
 CFLAGS = -O2 -Wall
+CPPFLAGS = -DNO_STRNDUP

 -include ../../../config.mak.autogen
 -include ../../../config.mak
diff --git a/contrib/credential/wincred/git-credential-wincred.c
b/contrib/credential/wincred/git-credential-wincred.c
index 721e59f..e26ba47 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -6,6 +6,7 @@
 #include <stdio.h>
 #include <io.h>
 #include <fcntl.h>
+#include <credential_helper.h>

 /* MinGW doesn't have wincred.h, so we need to define stuff */

@@ -124,9 +125,8 @@ static int prepare_credential(struct credential *c)
 		wusername = utf8_to_utf16_dup(c->username);
 	if (c->password)
 		wpassword = utf8_to_utf16_dup(c->password);
-	if (c->port) {
-		snprintf(port_buf,"%hd",c->port);
-	}
+	if (c->port)
+		snprintf(port_buf, sizeof(port_buf), "%hd", c->port);
 	return EXIT_SUCCESS;
 }

@@ -170,7 +170,7 @@ static int get_credential(struct credential *c)

 	/* search for the first credential that matches username */
 	for (i = 0; i < num_creds; ++i)
-		if (match_cred(creds[i])) {
+		if (match_cred(creds[i], c)) {
 			cred = creds[i];
 			break;
 		}
---8<---

However, I haven't been able to successfully run the tests on the
result. When I did, I got this error:

---8<---
rm: cannot remove `t/trash directory.t0303-credential-external/stderr': Permissi
on denied
rm: cannot remove `t/trash directory.t0303-credential-external/stdout': Permissi
on denied
rm: cannot remove directory `t/trash directory.t0303-credential-external': Direc
tory not empty
---8<---

And I'm not currently feeling up to debugging stuck processes or whatever it is.

Re: [PATCH 5/4] wincred: port to generic credential helper (UNTESTED)

[Junio C Hamano]

Erik Faye-Lund <kusmabite@gmail.com> writes:

> On Mon, Aug 27, 2012 at 12:04 AM, Philipp A. Hartmann <pah@qo.cx> wrote:
>> From: "Philipp A. Hartmann" <pah@qo.cx>
>>
>> This patch is an experiment to port the wincred helper
>> to the generic implementation.  As of know, it is
>> completely untested.
>>
>> In addition to porting the helper to the generic API,
>> this patch clears up all passwords from memory, which
>> reduces the total amount to saved lines.
>>
>> Signed-off-by: Philipp A. Hartmann <pah@qo.cx>
>> ---
>>
>> The porting was fairly easy, but due to the lack of a testing
>> platform, it is completely untested.
>>
>> Erik: Can you try to have look?
>
> Sorry for the late reply, I'm currently in bed with pneumonia.
>
> But I gave it a quick go, but as-is it's a NACK; a wall of warnings and errors.

Thanks, and get well soon.

For now, let's not worry about merging the "let's share code across
helpers" bit that comes later in Philipp's series.

Re: [PATCH 5/4] wincred: port to generic credential helper (UNTESTED)

[Erik Faye-Lund]

On Thu, Aug 30, 2012 at 8:27 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Mon, Aug 27, 2012 at 12:04 AM, Philipp A. Hartmann <pah@qo.cx> wrote:
>> From: "Philipp A. Hartmann" <pah@qo.cx>
>>
>> This patch is an experiment to port the wincred helper
>> to the generic implementation.  As of know, it is
>> completely untested.
>>
>> In addition to porting the helper to the generic API,
>> this patch clears up all passwords from memory, which
>> reduces the total amount to saved lines.
>>
>> Signed-off-by: Philipp A. Hartmann <pah@qo.cx>
>> ---
>>
>> The porting was fairly easy, but due to the lack of a testing
>> platform, it is completely untested.
>>
>> Erik: Can you try to have look?
>
> Sorry for the late reply, I'm currently in bed with pneumonia.
>
> But I gave it a quick go, but as-is it's a NACK; a wall of warnings and errors.
>
> But with this patch on top, it seems to at least compile OK:
>
> ---8<---
> diff --git a/contrib/credential/helper/credential_helper.h
> b/contrib/credential/helper/credential_helper.h
> index 7e73fc6..13b611e 100644
> --- a/contrib/credential/helper/credential_helper.h
> +++ b/contrib/credential/helper/credential_helper.h
> @@ -125,6 +125,7 @@ static inline char *xstrdup(const char *str)
>         return ret;
>  }
>
> +#ifndef NO_STRNDUP
>  static inline char *xstrndup(const char *str, size_t len)
>  {
>         char *ret = strndup(str,len);
> @@ -133,5 +134,6 @@ static inline char *xstrndup(const char *str, size_t len)
>
>         return ret;
>  }
> +#endif
>
>  #endif /* CREDENTIAL_HELPER_H_INCLUDED_ */
> diff --git a/contrib/credential/wincred/Makefile
> b/contrib/credential/wincred/Makefile
> index ee7a8ef..3900322 100644
> --- a/contrib/credential/wincred/Makefile
> +++ b/contrib/credential/wincred/Makefile
> @@ -1,9 +1,10 @@
>  MAIN:=git-credential-wincred
> -all:: $(MAIN)
> +all:: $(MAIN).exe
>
>  CC = gcc
>  RM = rm -f
>  CFLAGS = -O2 -Wall
> +CPPFLAGS = -DNO_STRNDUP
>
>  -include ../../../config.mak.autogen
>  -include ../../../config.mak
> diff --git a/contrib/credential/wincred/git-credential-wincred.c
> b/contrib/credential/wincred/git-credential-wincred.c
> index 721e59f..e26ba47 100644
> --- a/contrib/credential/wincred/git-credential-wincred.c
> +++ b/contrib/credential/wincred/git-credential-wincred.c
> @@ -6,6 +6,7 @@
>  #include <stdio.h>
>  #include <io.h>
>  #include <fcntl.h>
> +#include <credential_helper.h>
>
>  /* MinGW doesn't have wincred.h, so we need to define stuff */
>
> @@ -124,9 +125,8 @@ static int prepare_credential(struct credential *c)
>                 wusername = utf8_to_utf16_dup(c->username);
>         if (c->password)
>                 wpassword = utf8_to_utf16_dup(c->password);
> -       if (c->port) {
> -               snprintf(port_buf,"%hd",c->port);
> -       }
> +       if (c->port)
> +               snprintf(port_buf, sizeof(port_buf), "%hd", c->port);
>         return EXIT_SUCCESS;
>  }
>
> @@ -170,7 +170,7 @@ static int get_credential(struct credential *c)
>
>         /* search for the first credential that matches username */
>         for (i = 0; i < num_creds; ++i)
> -               if (match_cred(creds[i])) {
> +               if (match_cred(creds[i], c)) {
>                         cred = creds[i];
>                         break;
>                 }
> ---8<---
>
> However, I haven't been able to successfully run the tests on the
> result. When I did, I got this error:
>
> ---8<---
> rm: cannot remove `t/trash directory.t0303-credential-external/stderr': Permissi
> on denied
> rm: cannot remove `t/trash directory.t0303-credential-external/stdout': Permissi
> on denied
> rm: cannot remove directory `t/trash directory.t0303-credential-external': Direc
> tory not empty
> ---8<---
>
> And I'm not currently feeling up to debugging stuck processes or whatever it is.

OK, that was due to a stuck process, and after killing it I got to
test properly. However, three tests fail now:


$ (cd t && GIT_TEST_CREDENTIAL_HELPER=wincred ./t0303-credential-external.sh)
ok 1 - helper (wincred) has no existing data
ok 2 - helper (wincred) stores password
not ok - 3 helper (wincred) can retrieve password
#
#                       check fill $HELPER <<-\EOF
#                       protocol=https
#                       host=example.com
#                       --
#                       protocol=https
#                       host=example.com
#                       username=store-user
#                       password=store-pass
#                       --
#                       EOF
#
ok 4 - helper (wincred) requires matching protocol
ok 5 - helper (wincred) requires matching host
ok 6 - helper (wincred) requires matching username
ok 7 - helper (wincred) requires matching path
ok 8 - helper (wincred) can forget host
not ok - 9 helper (wincred) can store multiple users
#
#                       check approve $HELPER <<-\EOF &&
#                       protocol=https
#                       host=example.com
#                       username=user1
#                       password=pass1
#                       EOF
#                       check approve $HELPER <<-\EOF &&
#                       protocol=https
#                       host=example.com
#                       username=user2
#                       password=pass2
#                       EOF
#                       check fill $HELPER <<-\EOF &&
#                       protocol=https
#                       host=example.com
#                       username=user1
#                       --
#                       protocol=https
#                       host=example.com
#                       username=user1
#                       password=pass1
#                       EOF
#                       check fill $HELPER <<-\EOF
#                       protocol=https
#                       host=example.com
#                       username=user2
#                       --
#                       protocol=https
#                       host=example.com
#                       username=user2
#                       password=pass2
#                       EOF
#
ok 10 - helper (wincred) can forget user
not ok - 11 helper (wincred) remembers other user
#
#                       check fill $HELPER <<-\EOF
#                       protocol=https
#                       host=example.com
#                       username=user2
#                       --
#                       protocol=https
#                       host=example.com
#                       username=user2
#                       password=pass2
#                       EOF
#
# skipping timeout tests (GIT_TEST_CREDENTIAL_HELPER_TIMEOUT not set)
# failed 3 among 11 test(s)
1..11

So, something else is up as well.

[PATCH 5/4 v3] wincred: port to generic credential helper

[Philipp A. Hartmann]

From: "Philipp A. Hartmann" <pah@qo.cx>

In addition to porting the helper to the generic API,
this patch clears up all passwords from memory, which
reduces the total amount to saved lines.

This version will now pass t0303 if you do

   GIT_TEST_CREDENTIAL_HELPER=wincred \
   ./t0303-credential-external.sh

Signed-off-by: Philipp A. Hartmann <pah@qo.cx>
---

Erik, thanks for testing!

I finally managed to test the wincred implementation against t0303
and found another stupid bug in the credential matching of the port:

  -      (c->port && match_attr(cred, L"git_port", port_buf)) &&
  +      (!c->port || match_attr(cred, L"git_port", port_buf)) &&

Together with the fixes from Erik, the tests (and additional manual tests
with explicit ports, etc) now pass.

If this makes the generic implementation useful enough, I'd re-roll at least
the patches #2-#5 (#1 is already merged to next)  to a new series by adding
the "final" generic helper in one step and adding the ports afterwards.

[Apologies for reposting, I somehow broke the "From:" header in my
 original mail.]

 contrib/credential/helper/credential_helper.c      |   41 ++-
 contrib/credential/helper/credential_helper.h      |   16 +-
 contrib/credential/wincred/.gitignore              |    1 +
 contrib/credential/wincred/Makefile                |   20 +-
 .../credential/wincred/git-credential-wincred.c    |  312 +++++++++-----------
 5 files changed, 214 insertions(+), 176 deletions(-)
 create mode 100644 contrib/credential/wincred/.gitignore

diff --git a/contrib/credential/helper/credential_helper.c b/contrib/credential/helper/credential_helper.c
index e99c2ec..15536a6 100644
--- a/contrib/credential/helper/credential_helper.c
+++ b/contrib/credential/helper/credential_helper.c
@@ -11,6 +11,11 @@
 
 #include <credential_helper.h>
 
+#ifdef WIN32
+#include <fcntl.h>
+#include <io.h>
+#endif
+
 void credential_init(struct credential *c)
 {
 	memset(c, 0, sizeof(*c));
@@ -23,6 +28,7 @@ void credential_clear(struct credential *c)
 	free(c->path);
 	free(c->username);
 	free_password(c->password);
+	free(c->url);
 
 	credential_init(c);
 }
@@ -79,6 +85,29 @@ int credential_read(struct credential *c)
 		 * learn new lines, and the helpers are updated to match.
 		 */
 	}
+
+	/* Rebuild URI from parts */
+	*buf = '\0';
+	if (c->protocol) {
+		strncat(buf, c->protocol, sizeof(buf));
+		strncat(buf, "://", sizeof(buf));
+	}
+	if (c->username) {
+		strncat(buf, c->username, sizeof(buf));
+		strncat(buf, "@", sizeof(buf));
+	}
+	if (c->host)
+		strncat(buf, c->host, sizeof(buf));
+	if (c->port) {
+		value = buf + strlen(buf);
+		snprintf(value, sizeof(buf)-(value-buf), ":%hd", c->port);
+	}
+	if (c->path) {
+		strncat(buf, "/", sizeof(buf));
+		strncat(buf, c->path, sizeof(buf));
+	}
+	c->url = xstrdup(buf);
+
 	return 0;
 }
 
@@ -126,6 +155,12 @@ int main(int argc, char *argv[])
 		goto out;
 	}
 
+#ifdef WIN32
+	/* git on Windows uses binary pipes to avoid CRLF-issues */
+	_setmode(_fileno(stdin), _O_BINARY);
+	_setmode(_fileno(stdout), _O_BINARY);
+#endif
+
 	/* lookup operation callback */
 	while(try_op->name && strcmp(argv[1], try_op->name))
 		try_op++;
@@ -138,11 +173,15 @@ int main(int argc, char *argv[])
 	if(ret)
 		goto out;
 
+	if (!cred.protocol || !(cred.host || cred.path)) {
+		ret = EXIT_FAILURE;
+		goto out;
+	}
+
 	/* perform credential operation */
 	ret = (*try_op->op)(&cred);
 
 	credential_write(&cred);
-
 out:
 	credential_clear(&cred);
 	return ret;
diff --git a/contrib/credential/helper/credential_helper.h b/contrib/credential/helper/credential_helper.h
index 76b6e50..b5ffa5b 100644
--- a/contrib/credential/helper/credential_helper.h
+++ b/contrib/credential/helper/credential_helper.h
@@ -25,10 +25,11 @@ struct credential
 	char          *path;
 	char          *username;
 	char          *password;
+	char          *url;     /* <protocol>://[username@][host[:port]][/path] */
 };
 
 #define CREDENTIAL_INIT \
-  { NULL,NULL,0,NULL,NULL,NULL }
+  { NULL,NULL,0,NULL,NULL,NULL,NULL }
 
 void credential_init(struct credential *c);
 void credential_clear(struct credential *c);
@@ -104,6 +105,17 @@ static inline void die_errno(int err)
 	exit(EXIT_FAILURE);
 }
 
+static inline void *xmalloc(size_t size)
+{
+  void *ret = malloc(size);
+	if (!ret && !size)
+		ret = malloc(1);
+	if (!ret)
+		 die_errno(errno);
+
+	return ret;
+}
+
 static inline char *xstrdup(const char *str)
 {
 	char *ret = strdup(str);
@@ -113,6 +125,7 @@ static inline char *xstrdup(const char *str)
 	return ret;
 }
 
+#ifndef NO_STRNDUP
 static inline char *xstrndup(const char *str, size_t len)
 {
 	char *ret = strndup(str,len);
@@ -121,5 +134,6 @@ static inline char *xstrndup(const char *str, size_t len)
 
 	return ret;
 }
+#endif
 
 #endif /* CREDENTIAL_HELPER_H_INCLUDED_ */
diff --git a/contrib/credential/wincred/.gitignore b/contrib/credential/wincred/.gitignore
new file mode 100644
index 0000000..4780e4e
--- /dev/null
+++ b/contrib/credential/wincred/.gitignore
@@ -0,0 +1 @@
+git-credential-wincred.exe
diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile
index bad45ca..3900322 100644
--- a/contrib/credential/wincred/Makefile
+++ b/contrib/credential/wincred/Makefile
@@ -1,14 +1,26 @@
-all: git-credential-wincred.exe
+MAIN:=git-credential-wincred
+all:: $(MAIN).exe
 
 CC = gcc
 RM = rm -f
 CFLAGS = -O2 -Wall
+CPPFLAGS = -DNO_STRNDUP
 
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
-git-credential-wincred.exe : git-credential-wincred.c
-	$(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@
+HELPER:=../helper
+VPATH +=$(HELPER)
+
+SRCS:=$(MAIN).c
+SRCS+=credential_helper.c
+OBJS:=$(SRCS:.c=.o)
+
+%.o: %.c
+	$(CC) $(CFLAGS) $(CPPFLAGS) -I$(HELPER) -o $@ -c $<
+
+$(MAIN).exe: $(OBJS)
+	$(LINK.o) $^ $(LOADLIBES) $(LDLIBS) -o $@
 
 clean:
-	$(RM) git-credential-wincred.exe
+	$(RM) $(MAIN).exe $(OBJS)
diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c
index cbaec5f..6dabeb7 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -6,37 +6,7 @@
 #include <stdio.h>
 #include <io.h>
 #include <fcntl.h>
-
-/* common helpers */
-
-static void die(const char *err, ...)
-{
-	char msg[4096];
-	va_list params;
-	va_start(params, err);
-	vsnprintf(msg, sizeof(msg), err, params);
-	fprintf(stderr, "%s\n", msg);
-	va_end(params);
-	exit(1);
-}
-
-static void *xmalloc(size_t size)
-{
-	void *ret = malloc(size);
-	if (!ret && !size)
-		ret = malloc(1);
-	if (!ret)
-		 die("Out of memory");
-	return ret;
-}
-
-static char *xstrdup(const char *str)
-{
-	char *ret = strdup(str);
-	if (!ret)
-		die("Out of memory");
-	return ret;
-}
+#include <credential_helper.h>
 
 /* MinGW doesn't have wincred.h, so we need to define stuff */
 
@@ -84,13 +54,16 @@ static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW;
 static CredFreeT CredFree;
 static CredDeleteWT CredDeleteW;
 
-static void load_cred_funcs(void)
+
+static int load_cred_funcs(void)
 {
 	/* load DLLs */
 	advapi = LoadLibrary("advapi32.dll");
 	credui = LoadLibrary("credui.dll");
-	if (!advapi || !credui)
-		die("failed to load DLLs");
+	if (!advapi || !credui) {
+		error("failed to load DLLs");
+		return EXIT_FAILURE;
+	}
 
 	/* get function pointers */
 	CredWriteW = (CredWriteWT)GetProcAddress(advapi, "CredWriteW");
@@ -104,28 +77,57 @@ static void load_cred_funcs(void)
 	CredDeleteW = (CredDeleteWT)GetProcAddress(advapi, "CredDeleteW");
 	if (!CredWriteW || !CredUnPackAuthenticationBufferW ||
 	    !CredEnumerateW || !CredPackAuthenticationBufferW || !CredFree ||
-	    !CredDeleteW)
-		die("failed to load functions");
+	    !CredDeleteW) {
+		error("failed to load functions");
+		return EXIT_FAILURE;
+	}
+	return EXIT_SUCCESS;
 }
 
 static char target_buf[1024];
-static char *protocol, *host, *path, *username;
-static WCHAR *wusername, *password, *target;
+static char port_buf[8];
+static WCHAR *wusername, *wpassword;
 
-static void write_item(const char *what, WCHAR *wbuf)
+static WCHAR *utf8_to_utf16_dup(const char *str)
 {
-	char *buf;
-	int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, NULL, 0, NULL,
-	    FALSE);
-	buf = xmalloc(len);
-
-	if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, buf, len, NULL, FALSE))
-		die("WideCharToMultiByte failed!");
-
-	printf("%s=", what);
-	fwrite(buf, 1, len - 1, stdout);
-	putchar('\n');
-	free(buf);
+	int wlen = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0);
+	WCHAR *wstr = xmalloc(sizeof(WCHAR) * wlen);
+	if (!MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, wlen))
+		die("MultiByteToWideChar failed");
+	return wstr;
+}
+
+static char *utf16_to_utf8_dup(const WCHAR *wstr)
+{
+	int len = WideCharToMultiByte(CP_UTF8, 0, wstr, -1, NULL, 0, NULL,
+		FALSE);
+	char *str = xmalloc(len);
+	if (!WideCharToMultiByte(CP_UTF8, 0, wstr, -1, str, len, NULL, FALSE))
+		die("WideCharToMultiByte failed");
+	return str;
+}
+
+static void free_wpassword(WCHAR *pass)
+{
+	WCHAR *w = pass;
+	if(!pass)
+		return;
+	while(*w) *w++ = L'\0';
+	free(pass);
+}
+
+static int prepare_credential(struct credential *c)
+{
+	if (load_cred_funcs() )
+		return EXIT_FAILURE;
+
+	if (c->username)
+		wusername = utf8_to_utf16_dup(c->username);
+	if (c->password)
+		wpassword = utf8_to_utf16_dup(c->password);
+	if (c->port)
+		snprintf(port_buf, sizeof(port_buf), "%hd", c->port);
+	return EXIT_SUCCESS;
 }
 
 static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword,
@@ -143,33 +145,37 @@ static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword,
 	return 0; /* not found */
 }
 
-static int match_cred(const CREDENTIALW *cred)
+static int match_cred(const CREDENTIALW *cred, const struct credential *c)
 {
 	return (!wusername || !wcscmp(wusername, cred->UserName)) &&
-	    match_attr(cred, L"git_protocol", protocol) &&
-	    match_attr(cred, L"git_host", host) &&
-	    match_attr(cred, L"git_path", path);
+	    match_attr(cred, L"git_protocol", c->protocol) &&
+	    match_attr(cred, L"git_host", c->host) &&
+	    (!c->port || match_attr(cred, L"git_port", port_buf)) &&
+	    match_attr(cred, L"git_path", c->path);
 }
 
-static void get_credential(void)
+static int get_credential(struct credential *c)
 {
-	WCHAR *user_buf, *pass_buf;
+	WCHAR *user_buf = NULL, *pass_buf = NULL;
 	DWORD user_buf_size = 0, pass_buf_size = 0;
 	CREDENTIALW **creds, *cred = NULL;
 	DWORD num_creds;
-	int i;
+	int i, ret = EXIT_SUCCESS;
+
+	if (prepare_credential(c))
+		return EXIT_FAILURE;
 
 	if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds))
-		return;
+		return EXIT_FAILURE;
 
 	/* search for the first credential that matches username */
 	for (i = 0; i < num_creds; ++i)
-		if (match_cred(creds[i])) {
+		if (match_cred(creds[i], c)) {
 			cred = creds[i];
 			break;
 		}
 	if (!cred)
-		return;
+		goto out;
 
 	CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
 	    cred->CredentialBlobSize, NULL, &user_buf_size, NULL, NULL,
@@ -180,20 +186,29 @@ static void get_credential(void)
 
 	if (!CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
 	    cred->CredentialBlobSize, user_buf, &user_buf_size, NULL, NULL,
-	    pass_buf, &pass_buf_size))
-		die("CredUnPackAuthenticationBuffer failed");
-
-	CredFree(creds);
+	    pass_buf, &pass_buf_size)) {
+		error("CredUnPackAuthenticationBuffer failed");
+		ret = EXIT_FAILURE;
+		goto out;
+	}
 
 	/* zero-terminate (sizes include zero-termination) */
 	user_buf[user_buf_size - 1] = L'\0';
 	pass_buf[pass_buf_size - 1] = L'\0';
 
-	write_item("username", user_buf);
-	write_item("password", pass_buf);
+	if (!c->username)
+		c->username = utf16_to_utf8_dup(user_buf);
+	free_password(c->password);
+	c->password = utf16_to_utf8_dup(pass_buf);
+
+out:
+	CredFree(creds);
 
 	free(user_buf);
-	free(pass_buf);
+	free(wusername);
+	free_wpassword(pass_buf);
+
+	return ret;
 }
 
 static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword,
@@ -205,29 +220,42 @@ static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword,
 	attr->Value = (LPBYTE)value;
 }
 
-static void store_credential(void)
+static int store_credential(struct credential *c)
 {
 	CREDENTIALW cred;
 	BYTE *auth_buf;
 	DWORD auth_buf_size = 0;
 	CREDENTIAL_ATTRIBUTEW attrs[CRED_MAX_ATTRIBUTES];
+	WCHAR *wtarget;
+	int ret = EXIT_SUCCESS;
 
-	if (!wusername || !password)
-		return;
+	if (prepare_credential(c))
+		return EXIT_FAILURE;
+
+	if (!wusername || !wpassword)
+		return EXIT_FAILURE;
+
+	/* prepare 'target', the unique key for the credential */
+	strncat(target_buf,"git:",sizeof(target_buf));
+	strncat(target_buf,c->url,sizeof(target_buf));
+	wtarget = utf8_to_utf16_dup(target_buf);
 
 	/* query buffer size */
-	CredPackAuthenticationBufferW(0, wusername, password,
+	CredPackAuthenticationBufferW(0, wusername, wpassword,
 	    NULL, &auth_buf_size);
 
 	auth_buf = xmalloc(auth_buf_size);
 
-	if (!CredPackAuthenticationBufferW(0, wusername, password,
-	    auth_buf, &auth_buf_size))
-		die("CredPackAuthenticationBuffer failed");
+	if (!CredPackAuthenticationBufferW(0, wusername, wpassword,
+	    auth_buf, &auth_buf_size)) {
+		error("CredPackAuthenticationBuffer failed");
+		ret = EXIT_FAILURE;
+		goto out;
+	}
 
 	cred.Flags = 0;
 	cred.Type = CRED_TYPE_GENERIC;
-	cred.TargetName = target;
+	cred.TargetName = wtarget;
 	cred.Comment = L"saved by git-credential-wincred";
 	cred.CredentialBlobSize = auth_buf_size;
 	cred.CredentialBlob = auth_buf;
@@ -237,121 +265,65 @@ static void store_credential(void)
 	cred.TargetAlias = NULL;
 	cred.UserName = wusername;
 
-	write_attr(attrs, L"git_protocol", protocol);
+	write_attr(attrs, L"git_protocol", c->protocol);
+
+	if (c->host) {
+		write_attr(attrs + cred.AttributeCount, L"git_host", c->host);
+		cred.AttributeCount++;
+	}
 
-	if (host) {
-		write_attr(attrs + cred.AttributeCount, L"git_host", host);
+	if (c->port) {
+		write_attr(attrs + cred.AttributeCount, L"git_port", port_buf);
 		cred.AttributeCount++;
 	}
 
-	if (path) {
-		write_attr(attrs + cred.AttributeCount, L"git_path", path);
+	if (c->path) {
+		write_attr(attrs + cred.AttributeCount, L"git_path", c->path);
 		cred.AttributeCount++;
 	}
 
-	if (!CredWriteW(&cred, 0))
-		die("CredWrite failed");
+	if (!CredWriteW(&cred, 0)) {
+		error("CredWrite failed");
+		ret = EXIT_FAILURE;
+	}
+
+out:
+	free(auth_buf);
+	free(wusername);
+	free_wpassword(wpassword);
+	free(wtarget);
+	return ret;
 }
 
-static void erase_credential(void)
+static int erase_credential(struct credential *c)
 {
 	CREDENTIALW **creds;
 	DWORD num_creds;
 	int i;
 
+	if (prepare_credential(c))
+		return EXIT_FAILURE;
+
 	if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds))
-		return;
+		return EXIT_FAILURE;
 
 	for (i = 0; i < num_creds; ++i) {
-		if (match_cred(creds[i]))
+		if (match_cred(creds[i],c))
 			CredDeleteW(creds[i]->TargetName, creds[i]->Type, 0);
 	}
 
 	CredFree(creds);
+	return EXIT_SUCCESS;
 }
 
-static WCHAR *utf8_to_utf16_dup(const char *str)
-{
-	int wlen = MultiByteToWideChar(CP_UTF8, 0, str, -1, NULL, 0);
-	WCHAR *wstr = xmalloc(sizeof(WCHAR) * wlen);
-	MultiByteToWideChar(CP_UTF8, 0, str, -1, wstr, wlen);
-	return wstr;
-}
-
-static void read_credential(void)
-{
-	char buf[1024];
-
-	while (fgets(buf, sizeof(buf), stdin)) {
-		char *v;
-
-		if (!strcmp(buf, "\n"))
-			break;
-		buf[strlen(buf)-1] = '\0';
-
-		v = strchr(buf, '=');
-		if (!v)
-			die("bad input: %s", buf);
-		*v++ = '\0';
-
-		if (!strcmp(buf, "protocol"))
-			protocol = xstrdup(v);
-		else if (!strcmp(buf, "host"))
-			host = xstrdup(v);
-		else if (!strcmp(buf, "path"))
-			path = xstrdup(v);
-		else if (!strcmp(buf, "username")) {
-			username = xstrdup(v);
-			wusername = utf8_to_utf16_dup(v);
-		} else if (!strcmp(buf, "password"))
-			password = utf8_to_utf16_dup(v);
-		else
-			die("unrecognized input");
-	}
-}
-
-int main(int argc, char *argv[])
+/*
+ * Table with helper operation callbacks, used by generic
+ * credential helper main function.
+ */
+struct credential_operation const credential_helper_ops[] =
 {
-	const char *usage =
-	    "Usage: git credential-wincred <get|store|erase>\n";
-
-	if (!argv[1])
-		die(usage);
-
-	/* git use binary pipes to avoid CRLF-issues */
-	_setmode(_fileno(stdin), _O_BINARY);
-	_setmode(_fileno(stdout), _O_BINARY);
-
-	read_credential();
-
-	load_cred_funcs();
-
-	if (!protocol || !(host || path))
-		return 0;
-
-	/* prepare 'target', the unique key for the credential */
-	strncat(target_buf, "git:", sizeof(target_buf));
-	strncat(target_buf, protocol, sizeof(target_buf));
-	strncat(target_buf, "://", sizeof(target_buf));
-	if (username) {
-		strncat(target_buf, username, sizeof(target_buf));
-		strncat(target_buf, "@", sizeof(target_buf));
-	}
-	if (host)
-		strncat(target_buf, host, sizeof(target_buf));
-	if (path) {
-		strncat(target_buf, "/", sizeof(target_buf));
-		strncat(target_buf, path, sizeof(target_buf));
-	}
-
-	target = utf8_to_utf16_dup(target_buf);
-
-	if (!strcmp(argv[1], "get"))
-		get_credential();
-	else if (!strcmp(argv[1], "store"))
-		store_credential();
-	else if (!strcmp(argv[1], "erase"))
-		erase_credential();
-	/* otherwise, ignore unknown action */
-	return 0;
-}
+	{ "get",   get_credential   },
+	{ "store", store_credential },
+	{ "erase", erase_credential },
+	CREDENTIAL_OP_END
+};
-- 
1.7.10.4