[PATCH v2 00/16] Make Gnome Credential helper more Gnome-y and support ancient distros

[PATCH v2 00/16] Make Gnome Credential helper more Gnome-y and support ancient distros

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

Mostly unchanged.

Inserts a patch to fix the style issues for block statements.
i.e. use "if ()" instead of "if()"

A couple early patches were reordered to improve logical flow.

Updated the comment in the last patch to hopefully improve clarity
wrt RHEL 4.X

The only functional change is in 14/16 "report failure to store".
We should accept GNOME_KEYRING_RESULT_CANCELLED as a successful
return and _not_ produce an error message.

Interdiff follows...

Brandon Casey (16):
  contrib/git-credential-gnome-keyring.c: remove unnecessary
    pre-declarations
  contrib/git-credential-gnome-keyring.c: remove unused die() function
  contrib/git-credential-gnome-keyring.c: *style* use "if ()" not "if()"
    etc.
  contrib/git-credential-gnome-keyring.c: add static where applicable
  contrib/git-credential-gnome-keyring.c: exit non-zero when called
    incorrectly
  contrib/git-credential-gnome-keyring.c: strlen() returns size_t, not
    ssize_t
  contrib/git-credential-gnome-keyring.c: ensure buffer is non-empty
    before accessing
  contrib/git-credential-gnome-keyring.c: set Gnome application name
  contrib/git-credential-gnome-keyring.c: use gnome helpers in
    keyring_object()
  contrib/git-credential-gnome-keyring.c: use secure memory functions
    for passwds
  contrib/git-credential-gnome-keyring.c: use secure memory for reading
    passwords
  contrib/git-credential-gnome-keyring.c: use glib memory allocation
    functions
  contrib/git-credential-gnome-keyring.c: use glib messaging functions
  contrib/git-credential-gnome-keyring.c: report failure to store
    password
  contrib/git-credential-gnome-keyring.c: support ancient gnome-keyring
  contrib/git-credential-gnome-keyring.c: support really ancient
    gnome-keyring

 contrib/credential/gnome-keyring/Makefile          |   4 +-
 .../gnome-keyring/git-credential-gnome-keyring.c   | 301 ++++++++++++---------
 2 files changed, 169 insertions(+), 136 deletions(-)

---

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index ce2ddee..635c96b 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -50,7 +50,7 @@
 
 /*
  * ancient gnome-keyring returns DENIED when an entry is not found.
- * Setting NO_MATCH to DENIED will prevent us from reporting denied
+ * Setting NO_MATCH to DENIED will prevent us from reporting DENIED
  * errors during get and erase operations, but we will still report
  * DENIED errors during a store.
  */
@@ -87,8 +87,8 @@ static const char* gnome_keyring_result_to_message(GnomeKeyringResult result)
 }
 
 /*
- * Just a guess to support RHEL 4.X.
- * Glib 2.8 was roughly Gnome 2.12 ?
+ * Support really ancient gnome-keyring, circ. RHEL 4.X.
+ * Just a guess for the Glib version.  Glib 2.8 was roughly Gnome 2.12 ?
  * Which was released with gnome-keyring 0.4.3 ??
  */
 #if GLIB_MAJOR_VERSION == 2 && GLIB_MINOR_VERSION < 8
@@ -162,7 +162,7 @@ static char* keyring_object(struct credential *c)
 	if (!c->path)
 		return NULL;
 
-	if(c->port)
+	if (c->port)
 		return g_strdup_printf("%s:%hd/%s", c->host, c->port, c->path);
 
 	return g_strdup_printf("%s/%s", c->host, c->path);
@@ -251,7 +251,8 @@ static int keyring_store(struct credential *c)
 
 	g_free(object);
 
-	if (result != GNOME_KEYRING_RESULT_OK) {
+	if (result != GNOME_KEYRING_RESULT_OK &&
+	    result != GNOME_KEYRING_RESULT_CANCELLED) {
 		g_critical("%s", gnome_keyring_result_to_message(result));
 		return EXIT_FAILURE;
 	}
@@ -363,14 +364,14 @@ static int credential_read(struct credential *c)
 	{
 		line_len = strlen(buf);
 
-		if(line_len && buf[line_len-1] == '\n')
+		if (line_len && buf[line_len-1] == '\n')
 			buf[--line_len]='\0';
 
-		if(!line_len)
+		if (!line_len)
 			break;
 
 		value = strchr(buf,'=');
-		if(!value) {
+		if (!value) {
 			g_warning("invalid credential line: %s", key);
 			gnome_keyring_memory_free(buf);
 			return -1;
@@ -432,9 +433,9 @@ static void usage(const char *name)
 
 	basename = (basename) ? basename + 1 : name;
 	fprintf(stderr, "usage: %s <", basename);
-	while(try_op->name) {
+	while (try_op->name) {
 		fprintf(stderr,"%s",(try_op++)->name);
-		if(try_op->name)
+		if (try_op->name)
 			fprintf(stderr,"%s","|");
 	}
 	fprintf(stderr,"%s",">\n");
@@ -455,15 +456,15 @@ int main(int argc, char *argv[])
 	g_set_application_name("Git Credential Helper");
 
 	/* lookup operation callback */
-	while(try_op->name && strcmp(argv[1], try_op->name))
+	while (try_op->name && strcmp(argv[1], try_op->name))
 		try_op++;
 
 	/* unsupported operation given -- ignore silently */
-	if(!try_op->name || !try_op->op)
+	if (!try_op->name || !try_op->op)
 		goto out;
 
 	ret = credential_read(&cred);
-	if(ret)
+	if (ret)
 		goto out;
 
 	/* perform credential operation */

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 01/16] contrib/git-credential-gnome-keyring.c: remove unnecessary pre-declarations

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

These are all defined before they are used, so it is not necessary to
pre-declare them.  Remove the pre-declarations.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 .../credential/gnome-keyring/git-credential-gnome-keyring.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index f2cdefe..15b0a1c 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -46,11 +46,6 @@ struct credential
 #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
@@ -62,14 +57,6 @@ struct credential_operation
 #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)
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 02/16] contrib/git-credential-gnome-keyring.c: remove unused die() function

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 .../credential/gnome-keyring/git-credential-gnome-keyring.c    | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 15b0a1c..4334f23 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -91,16 +91,6 @@ static inline void error(const char *fmt, ...)
 	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));
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 03/16] contrib/git-credential-gnome-keyring.c: *style* use "if ()" not "if()" etc.

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 .../gnome-keyring/git-credential-gnome-keyring.c     | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 4334f23..809b1b7 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -117,10 +117,10 @@ static char* keyring_object(struct credential *c)
 		return object;
 
 	object = (char*) malloc(strlen(c->host)+strlen(c->path)+8);
-	if(!object)
+	if (!object)
 		die_errno(errno);
 
-	if(c->port)
+	if (c->port)
 		sprintf(object,"%s:%hd/%s",c->host,c->port,c->path);
 	else
 		sprintf(object,"%s/%s",c->host,c->path);
@@ -314,14 +314,14 @@ int credential_read(struct credential *c)
 	{
 		line_len = strlen(buf);
 
-		if(buf[line_len-1]=='\n')
+		if (buf[line_len-1]=='\n')
 			buf[--line_len]='\0';
 
-		if(!line_len)
+		if (!line_len)
 			break;
 
 		value = strchr(buf,'=');
-		if(!value) {
+		if (!value) {
 			warning("invalid credential line: %s", key);
 			return -1;
 		}
@@ -379,9 +379,9 @@ static void usage(const char *name)
 
 	basename = (basename) ? basename + 1 : name;
 	fprintf(stderr, "usage: %s <", basename);
-	while(try_op->name) {
+	while (try_op->name) {
 		fprintf(stderr,"%s",(try_op++)->name);
-		if(try_op->name)
+		if (try_op->name)
 			fprintf(stderr,"%s","|");
 	}
 	fprintf(stderr,"%s",">\n");
@@ -400,15 +400,15 @@ int main(int argc, char *argv[])
 	}
 
 	/* lookup operation callback */
-	while(try_op->name && strcmp(argv[1], try_op->name))
+	while (try_op->name && strcmp(argv[1], try_op->name))
 		try_op++;
 
 	/* unsupported operation given -- ignore silently */
-	if(!try_op->name || !try_op->op)
+	if (!try_op->name || !try_op->op)
 		goto out;
 
 	ret = credential_read(&cred);
-	if(ret)
+	if (ret)
 		goto out;
 
 	/* perform credential operation */
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 04/16] contrib/git-credential-gnome-keyring.c: add static where applicable

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

Mark global variable and functions as static.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 .../gnome-keyring/git-credential-gnome-keyring.c       | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 809b1b7..3ff541c 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -128,7 +128,7 @@ static char* keyring_object(struct credential *c)
 	return object;
 }
 
-int keyring_get(struct credential *c)
+static int keyring_get(struct credential *c)
 {
 	char* object = NULL;
 	GList *entries;
@@ -178,7 +178,7 @@ int keyring_get(struct credential *c)
 }
 
 
-int keyring_store(struct credential *c)
+static int keyring_store(struct credential *c)
 {
 	guint32 item_id;
 	char  *object = NULL;
@@ -212,7 +212,7 @@ int keyring_store(struct credential *c)
 	return EXIT_SUCCESS;
 }
 
-int keyring_erase(struct credential *c)
+static int keyring_erase(struct credential *c)
 {
 	char  *object = NULL;
 	GList *entries;
@@ -277,7 +277,7 @@ int keyring_erase(struct credential *c)
  * Table with helper operation callbacks, used by generic
  * credential helper main function.
  */
-struct credential_operation const credential_helper_ops[] =
+static struct credential_operation const credential_helper_ops[] =
 {
 	{ "get",   keyring_get   },
 	{ "store", keyring_store },
@@ -287,12 +287,12 @@ struct credential_operation const credential_helper_ops[] =
 
 /* ------------------ credential functions ------------------ */
 
-void credential_init(struct credential *c)
+static void credential_init(struct credential *c)
 {
 	memset(c, 0, sizeof(*c));
 }
 
-void credential_clear(struct credential *c)
+static void credential_clear(struct credential *c)
 {
 	free(c->protocol);
 	free(c->host);
@@ -303,7 +303,7 @@ void credential_clear(struct credential *c)
 	credential_init(c);
 }
 
-int credential_read(struct credential *c)
+static int credential_read(struct credential *c)
 {
 	char    buf[1024];
 	ssize_t line_len = 0;
@@ -358,14 +358,14 @@ int credential_read(struct credential *c)
 	return 0;
 }
 
-void credential_write_item(FILE *fp, const char *key, const char *value)
+static 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)
+static void credential_write(const struct credential *c)
 {
 	/* only write username/password, if set */
 	credential_write_item(stdout, "username", c->username);
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 05/16] contrib/git-credential-gnome-keyring.c: exit non-zero when called incorrectly

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

If the correct arguments were not specified, this program should exit
non-zero.  Let's do so.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 contrib/credential/gnome-keyring/git-credential-gnome-keyring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 3ff541c..04852d7 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -396,7 +396,7 @@ int main(int argc, char *argv[])
 
 	if (!argv[1]) {
 		usage(argv[0]);
-		goto out;
+		exit(EXIT_FAILURE);
 	}
 
 	/* lookup operation callback */
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 06/16] contrib/git-credential-gnome-keyring.c: strlen() returns size_t, not ssize_t

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

Also, initialization is not necessary since it is assigned before it is
used.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 contrib/credential/gnome-keyring/git-credential-gnome-keyring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 04852d7..b9bb794 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -306,7 +306,7 @@ static void credential_clear(struct credential *c)
 static int credential_read(struct credential *c)
 {
 	char    buf[1024];
-	ssize_t line_len = 0;
+	size_t line_len;
 	char   *key      = buf;
 	char   *value;
 
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 07/16] contrib/git-credential-gnome-keyring.c: ensure buffer is non-empty before accessing

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

Ensure buffer length is non-zero before attempting to access the last
element.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 contrib/credential/gnome-keyring/git-credential-gnome-keyring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index b9bb794..0d2c55e 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -314,7 +314,7 @@ static int credential_read(struct credential *c)
 	{
 		line_len = strlen(buf);
 
-		if (buf[line_len-1]=='\n')
+		if (line_len && buf[line_len-1] == '\n')
 			buf[--line_len]='\0';
 
 		if (!line_len)
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 08/16] contrib/git-credential-gnome-keyring.c: set Gnome application name

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

Since this is a Gnome application, let's set the application name to
something reasonable.  This will be displayed in Gnome dialog boxes
e.g. the one that prompts for the user's keyring password.

We add an include statement for glib.h and add the glib-2.0 cflags and
libs to the compilation arguments, but both of these are really noops
since glib is already a dependency of gnome-keyring.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 contrib/credential/gnome-keyring/Makefile                       | 4 ++--
 contrib/credential/gnome-keyring/git-credential-gnome-keyring.c | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/contrib/credential/gnome-keyring/Makefile b/contrib/credential/gnome-keyring/Makefile
index e6561d8..c3c7c98 100644
--- a/contrib/credential/gnome-keyring/Makefile
+++ b/contrib/credential/gnome-keyring/Makefile
@@ -8,8 +8,8 @@ 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)
+INCS:=$(shell pkg-config --cflags gnome-keyring-1 glib-2.0)
+LIBS:=$(shell pkg-config --libs gnome-keyring-1 glib-2.0)
 
 SRCS:=$(MAIN).c
 OBJS:=$(SRCS:.c=.o)
diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 0d2c55e..43b19dd 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -28,6 +28,7 @@
 #include <stdarg.h>
 #include <stdlib.h>
 #include <errno.h>
+#include <glib.h>
 #include <gnome-keyring.h>
 
 /*
@@ -399,6 +400,8 @@ int main(int argc, char *argv[])
 		exit(EXIT_FAILURE);
 	}
 
+	g_set_application_name("Git Credential Helper");
+
 	/* lookup operation callback */
 	while (try_op->name && strcmp(argv[1], try_op->name))
 		try_op++;
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 09/16] contrib/git-credential-gnome-keyring.c: use gnome helpers in keyring_object()

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

Rather than carefully allocating memory for sprintf() to write into,
let's make use of the glib helper function g_strdup_printf(), which
makes things a lot easier and less error-prone.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 .../gnome-keyring/git-credential-gnome-keyring.c           | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 43b19dd..b692e1f 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -112,21 +112,13 @@ static inline char *xstrdup(const char *str)
 /* 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);
+		return NULL;
 
 	if (c->port)
-		sprintf(object,"%s:%hd/%s",c->host,c->port,c->path);
-	else
-		sprintf(object,"%s/%s",c->host,c->path);
+		return g_strdup_printf("%s:%hd/%s", c->host, c->port, c->path);
 
-	return object;
+	return g_strdup_printf("%s/%s", c->host, c->path);
 }
 
 static int keyring_get(struct credential *c)
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 10/16] contrib/git-credential-gnome-keyring.c: use secure memory functions for passwds

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

gnome-keyring provides functions for allocating non-pageable memory (if
possible) intended to be used for storing passwords.  Let's use them.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 .../gnome-keyring/git-credential-gnome-keyring.c    | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index b692e1f..d8a7038 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -30,6 +30,7 @@
 #include <errno.h>
 #include <glib.h>
 #include <gnome-keyring.h>
+#include <gnome-keyring-memory.h>
 
 /*
  * This credential struct and API is simplified from git's credential.{h,c}
@@ -60,16 +61,6 @@ struct credential_operation
 
 /* ---------------- 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;
@@ -159,8 +150,8 @@ static int keyring_get(struct credential *c)
 	/* pick the first one from the list */
 	password_data = (GnomeKeyringNetworkPasswordData *) entries->data;
 
-	free_password(c->password);
-	c->password = xstrdup(password_data->password);
+	gnome_keyring_memory_free(c->password);
+	c->password = gnome_keyring_memory_strdup(password_data->password);
 
 	if (!c->username)
 		c->username = xstrdup(password_data->user);
@@ -291,7 +282,7 @@ static void credential_clear(struct credential *c)
 	free(c->host);
 	free(c->path);
 	free(c->username);
-	free_password(c->password);
+	gnome_keyring_memory_free(c->password);
 
 	credential_init(c);
 }
@@ -338,8 +329,8 @@ static int credential_read(struct credential *c)
 			free(c->username);
 			c->username = xstrdup(value);
 		} else if (!strcmp(key, "password")) {
-			free_password(c->password);
-			c->password = xstrdup(value);
+			gnome_keyring_memory_free(c->password);
+			c->password = gnome_keyring_memory_strdup(value);
 			while (*value) *value++ = '\0';
 		}
 		/*
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 11/16] contrib/git-credential-gnome-keyring.c: use secure memory for reading passwords

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

gnome-keyring provides functions to allocate non-pageable memory (if
possible).  Let's use them to allocate memory that may be used to hold
secure data read from the keyring.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 .../credential/gnome-keyring/git-credential-gnome-keyring.c  | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index d8a7038..5e79669 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -289,12 +289,14 @@ static void credential_clear(struct credential *c)
 
 static int credential_read(struct credential *c)
 {
-	char    buf[1024];
+	char    *buf;
 	size_t line_len;
-	char   *key      = buf;
+	char   *key;
 	char   *value;
 
-	while (fgets(buf, sizeof(buf), stdin))
+	key = buf = gnome_keyring_memory_alloc(1024);
+
+	while (fgets(buf, 1024, stdin))
 	{
 		line_len = strlen(buf);
 
@@ -307,6 +309,7 @@ static int credential_read(struct credential *c)
 		value = strchr(buf,'=');
 		if (!value) {
 			warning("invalid credential line: %s", key);
+			gnome_keyring_memory_free(buf);
 			return -1;
 		}
 		*value++ = '\0';
@@ -339,6 +342,9 @@ static int credential_read(struct credential *c)
 		 * learn new lines, and the helpers are updated to match.
 		 */
 	}
+
+	gnome_keyring_memory_free(buf);
+
 	return 0;
 }
 
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 12/16] contrib/git-credential-gnome-keyring.c: use glib memory allocation functions

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

Rather than roll our own, let's use the memory allocation/free routines
provided by glib.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 .../gnome-keyring/git-credential-gnome-keyring.c   | 48 ++++++++--------------
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 5e79669..273c43b 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -27,7 +27,6 @@
 #include <string.h>
 #include <stdarg.h>
 #include <stdlib.h>
-#include <errno.h>
 #include <glib.h>
 #include <gnome-keyring.h>
 #include <gnome-keyring-memory.h>
@@ -83,21 +82,6 @@ static inline void error(const char *fmt, ...)
 	va_end(ap);
 }
 
-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 */
@@ -134,7 +118,7 @@ static int keyring_get(struct credential *c)
 				c->port,
 				&entries);
 
-	free(object);
+	g_free(object);
 
 	if (result == GNOME_KEYRING_RESULT_NO_MATCH)
 		return EXIT_SUCCESS;
@@ -154,7 +138,7 @@ static int keyring_get(struct credential *c)
 	c->password = gnome_keyring_memory_strdup(password_data->password);
 
 	if (!c->username)
-		c->username = xstrdup(password_data->user);
+		c->username = g_strdup(password_data->user);
 
 	gnome_keyring_network_password_list_free(entries);
 
@@ -192,7 +176,7 @@ static int keyring_store(struct credential *c)
 				c->password,
 				&item_id);
 
-	free(object);
+	g_free(object);
 	return EXIT_SUCCESS;
 }
 
@@ -226,7 +210,7 @@ static int keyring_erase(struct credential *c)
 				c->port,
 				&entries);
 
-	free(object);
+	g_free(object);
 
 	if (result == GNOME_KEYRING_RESULT_NO_MATCH)
 		return EXIT_SUCCESS;
@@ -278,10 +262,10 @@ static void credential_init(struct credential *c)
 
 static void credential_clear(struct credential *c)
 {
-	free(c->protocol);
-	free(c->host);
-	free(c->path);
-	free(c->username);
+	g_free(c->protocol);
+	g_free(c->host);
+	g_free(c->path);
+	g_free(c->username);
 	gnome_keyring_memory_free(c->password);
 
 	credential_init(c);
@@ -315,22 +299,22 @@ static int credential_read(struct credential *c)
 		*value++ = '\0';
 
 		if (!strcmp(key, "protocol")) {
-			free(c->protocol);
-			c->protocol = xstrdup(value);
+			g_free(c->protocol);
+			c->protocol = g_strdup(value);
 		} else if (!strcmp(key, "host")) {
-			free(c->host);
-			c->host = xstrdup(value);
+			g_free(c->host);
+			c->host = g_strdup(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);
+			g_free(c->path);
+			c->path = g_strdup(value);
 		} else if (!strcmp(key, "username")) {
-			free(c->username);
-			c->username = xstrdup(value);
+			g_free(c->username);
+			c->username = g_strdup(value);
 		} else if (!strcmp(key, "password")) {
 			gnome_keyring_memory_free(c->password);
 			c->password = gnome_keyring_memory_strdup(value);
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 13/16] contrib/git-credential-gnome-keyring.c: use glib messaging functions

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

Rather than roll our own, let's use the messaging functions provided
by glib.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 .../gnome-keyring/git-credential-gnome-keyring.c   | 33 +++-------------------
 1 file changed, 4 insertions(+), 29 deletions(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 273c43b..b70bd53 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -25,7 +25,6 @@
 
 #include <stdio.h>
 #include <string.h>
-#include <stdarg.h>
 #include <stdlib.h>
 #include <glib.h>
 #include <gnome-keyring.h>
@@ -58,30 +57,6 @@ struct credential_operation
 #define CREDENTIAL_OP_END \
   { NULL,NULL }
 
-/* ---------------- common helper functions ----------------- */
-
-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);
-}
-
 /* ----------------- GNOME Keyring functions ----------------- */
 
 /* create a special keyring option string, if path is given */
@@ -127,7 +102,7 @@ static int keyring_get(struct credential *c)
 		return EXIT_SUCCESS;
 
 	if (result != GNOME_KEYRING_RESULT_OK) {
-		error("%s",gnome_keyring_result_to_message(result));
+		g_critical("%s", gnome_keyring_result_to_message(result));
 		return EXIT_FAILURE;
 	}
 
@@ -220,7 +195,7 @@ static int keyring_erase(struct credential *c)
 
 	if (result != GNOME_KEYRING_RESULT_OK)
 	{
-		error("%s",gnome_keyring_result_to_message(result));
+		g_critical("%s", gnome_keyring_result_to_message(result));
 		return EXIT_FAILURE;
 	}
 
@@ -234,7 +209,7 @@ static int keyring_erase(struct credential *c)
 
 	if (result != GNOME_KEYRING_RESULT_OK)
 	{
-		error("%s",gnome_keyring_result_to_message(result));
+		g_critical("%s", gnome_keyring_result_to_message(result));
 		return EXIT_FAILURE;
 	}
 
@@ -292,7 +267,7 @@ static int credential_read(struct credential *c)
 
 		value = strchr(buf,'=');
 		if (!value) {
-			warning("invalid credential line: %s", key);
+			g_warning("invalid credential line: %s", key);
 			gnome_keyring_memory_free(buf);
 			return -1;
 		}
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 14/16] contrib/git-credential-gnome-keyring.c: report failure to store password

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

Produce an error message when we fail to store a password to the keyring.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---

Difference from v1:
Additionally interpret GNOME_KEYRING_RESULT_CANCELLED as a successful exit
status, since that means that the user intentionally cancelled the
operation from the gui.  We should not produce a warning in that case.

 .../credential/gnome-keyring/git-credential-gnome-keyring.c    | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index b70bd53..447e9aa 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -125,6 +125,7 @@ static int keyring_store(struct credential *c)
 {
 	guint32 item_id;
 	char  *object = NULL;
+	GnomeKeyringResult result;
 
 	/*
 	 * Sanity check that what we are storing is actually sensible.
@@ -139,7 +140,7 @@ static int keyring_store(struct credential *c)
 
 	object = keyring_object(c);
 
-	gnome_keyring_set_network_password_sync(
+	result = gnome_keyring_set_network_password_sync(
 				GNOME_KEYRING_DEFAULT,
 				c->username,
 				NULL /* domain */,
@@ -152,6 +153,13 @@ static int keyring_store(struct credential *c)
 				&item_id);
 
 	g_free(object);
+
+	if (result != GNOME_KEYRING_RESULT_OK &&
+	    result != GNOME_KEYRING_RESULT_CANCELLED) {
+		g_critical("%s", gnome_keyring_result_to_message(result));
+		return EXIT_FAILURE;
+	}
+
 	return EXIT_SUCCESS;
 }
 
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 15/16] contrib/git-credential-gnome-keyring.c: support ancient gnome-keyring

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

The gnome-keyring lib distributed with RHEL 5.X is ancient and does
not provide a few of the functions/defines that more recent versions
do, but mostly the API is the same.  Let's provide the missing bits
via macro definitions and function implementation.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 .../gnome-keyring/git-credential-gnome-keyring.c   | 58 ++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 447e9aa..e1bc3fa 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -28,8 +28,66 @@
 #include <stdlib.h>
 #include <glib.h>
 #include <gnome-keyring.h>
+
+#ifdef GNOME_KEYRING_DEFAULT
+
+   /* Modern gnome-keyring */
+
 #include <gnome-keyring-memory.h>
 
+#else
+
+   /*
+    * Support ancient gnome-keyring, circ. RHEL 5.X.
+    * GNOME_KEYRING_DEFAULT seems to have been introduced with Gnome 2.22,
+    * and the other features roughly around Gnome 2.20, 6 months before.
+    * Ubuntu 8.04 used Gnome 2.22 (I think).  Not sure any distro used 2.20.
+    * So the existence/non-existence of GNOME_KEYRING_DEFAULT seems like
+    * a decent thing to use as an indicator.
+    */
+
+#define GNOME_KEYRING_DEFAULT NULL
+
+/*
+ * ancient gnome-keyring returns DENIED when an entry is not found.
+ * Setting NO_MATCH to DENIED will prevent us from reporting DENIED
+ * errors during get and erase operations, but we will still report
+ * DENIED errors during a store.
+ */
+#define GNOME_KEYRING_RESULT_NO_MATCH GNOME_KEYRING_RESULT_DENIED
+
+#define gnome_keyring_memory_alloc g_malloc
+#define gnome_keyring_memory_free gnome_keyring_free_password
+#define gnome_keyring_memory_strdup g_strdup
+
+static const char* gnome_keyring_result_to_message(GnomeKeyringResult result)
+{
+	switch (result) {
+	case GNOME_KEYRING_RESULT_OK:
+		return "OK";
+	case GNOME_KEYRING_RESULT_DENIED:
+		return "Denied";
+	case GNOME_KEYRING_RESULT_NO_KEYRING_DAEMON:
+		return "No Keyring Daemon";
+	case GNOME_KEYRING_RESULT_ALREADY_UNLOCKED:
+		return "Already UnLocked";
+	case GNOME_KEYRING_RESULT_NO_SUCH_KEYRING:
+		return "No Such Keyring";
+	case GNOME_KEYRING_RESULT_BAD_ARGUMENTS:
+		return "Bad Arguments";
+	case GNOME_KEYRING_RESULT_IO_ERROR:
+		return "IO Error";
+	case GNOME_KEYRING_RESULT_CANCELLED:
+		return "Cancelled";
+	case GNOME_KEYRING_RESULT_ALREADY_EXISTS:
+		return "Already Exists";
+	default:
+		return "Unknown Error";
+	}
+}
+
+#endif
+
 /*
  * This credential struct and API is simplified from git's credential.{h,c}
  */
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[PATCH v2 16/16] contrib/git-credential-gnome-keyring.c: support really ancient gnome-keyring

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

The gnome-keyring lib (0.4) distributed with RHEL 4.X is really ancient
and does not provide most of the synchronous functions that even ancient
releases do.  Thankfully, we're only using one function that is missing.
Let's emulate gnome_keyring_item_delete_sync() by calling the asynchronous
function and then triggering the event loop processing until our
callback is called.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---

Clarified the comment about RHEL 4.X support.

 .../gnome-keyring/git-credential-gnome-keyring.c   | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index e1bc3fa..635c96b 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -86,6 +86,45 @@ static const char* gnome_keyring_result_to_message(GnomeKeyringResult result)
 	}
 }
 
+/*
+ * Support really ancient gnome-keyring, circ. RHEL 4.X.
+ * Just a guess for the Glib version.  Glib 2.8 was roughly Gnome 2.12 ?
+ * Which was released with gnome-keyring 0.4.3 ??
+ */
+#if GLIB_MAJOR_VERSION == 2 && GLIB_MINOR_VERSION < 8
+
+static void gnome_keyring_done_cb(GnomeKeyringResult result, gpointer user_data)
+{
+	gpointer *data = (gpointer*) user_data;
+	int *done = (int*) data[0];
+	GnomeKeyringResult *r = (GnomeKeyringResult*) data[1];
+
+	*r = result;
+	*done = 1;
+}
+
+static void wait_for_request_completion(int *done)
+{
+	GMainContext *mc = g_main_context_default();
+	while (!*done)
+		g_main_context_iteration(mc, TRUE);
+}
+
+static GnomeKeyringResult gnome_keyring_item_delete_sync(const char *keyring, guint32 id)
+{
+	int done = 0;
+	GnomeKeyringResult result;
+	gpointer data[] = { &done, &result };
+
+	gnome_keyring_item_delete(keyring, id, gnome_keyring_done_cb, data,
+		NULL);
+
+	wait_for_request_completion(&done);
+
+	return result;
+}
+
+#endif
 #endif
 
 /*
-- 
1.8.4.rc4.6.g5555d19


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH v2 15/16] contrib/git-credential-gnome-keyring.c: support ancient gnome-keyring

[Felipe Contreras]

On Mon, Sep 23, 2013 at 1:49 PM, Brandon Casey <bcasey@nvidia.com> wrote:
> From: Brandon Casey <drafnel@gmail.com>
>
> The gnome-keyring lib distributed with RHEL 5.X is ancient and does
> not provide a few of the functions/defines that more recent versions
> do, but mostly the API is the same.  Let's provide the missing bits
> via macro definitions and function implementation.
>
> Signed-off-by: Brandon Casey <drafnel@gmail.com>
> ---
>  .../gnome-keyring/git-credential-gnome-keyring.c   | 58 ++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>
> diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
> index 447e9aa..e1bc3fa 100644
> --- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
> +++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
> @@ -28,8 +28,66 @@
>  #include <stdlib.h>
>  #include <glib.h>
>  #include <gnome-keyring.h>
> +
> +#ifdef GNOME_KEYRING_DEFAULT
> +
> +   /* Modern gnome-keyring */
> +
>  #include <gnome-keyring-memory.h>
>
> +#else
> +
> +   /*
> +    * Support ancient gnome-keyring, circ. RHEL 5.X.
> +    * GNOME_KEYRING_DEFAULT seems to have been introduced with Gnome 2.22,
> +    * and the other features roughly around Gnome 2.20, 6 months before.
> +    * Ubuntu 8.04 used Gnome 2.22 (I think).  Not sure any distro used 2.20.
> +    * So the existence/non-existence of GNOME_KEYRING_DEFAULT seems like
> +    * a decent thing to use as an indicator.
> +    */
> +
> +#define GNOME_KEYRING_DEFAULT NULL
> +
> +/*
> + * ancient gnome-keyring returns DENIED when an entry is not found.
> + * Setting NO_MATCH to DENIED will prevent us from reporting DENIED
> + * errors during get and erase operations, but we will still report
> + * DENIED errors during a store.
> + */
> +#define GNOME_KEYRING_RESULT_NO_MATCH GNOME_KEYRING_RESULT_DENIED
> +
> +#define gnome_keyring_memory_alloc g_malloc
> +#define gnome_keyring_memory_free gnome_keyring_free_password
> +#define gnome_keyring_memory_strdup g_strdup
> +
> +static const char* gnome_keyring_result_to_message(GnomeKeyringResult result)

The style is:
static const char *gnome_keyring_result_to_message

There might not be a need to reroll, depending on what Junio says.

-- 
Felipe Contreras

Re: [PATCH v2 00/16] Make Gnome Credential helper more Gnome-y and support ancient distros

[Felipe Contreras]

On Mon, Sep 23, 2013 at 1:49 PM, Brandon Casey <bcasey@nvidia.com> wrote:
> From: Brandon Casey <drafnel@gmail.com>
>
> Mostly unchanged.
>
> Inserts a patch to fix the style issues for block statements.
> i.e. use "if ()" instead of "if()"
>
> A couple early patches were reordered to improve logical flow.
>
> Updated the comment in the last patch to hopefully improve clarity
> wrt RHEL 4.X
>
> The only functional change is in 14/16 "report failure to store".
> We should accept GNOME_KEYRING_RESULT_CANCELLED as a successful
> return and _not_ produce an error message.
>
> Interdiff follows...

I'm familiar with GLib and I looked at the patches, they look good to me.

-- 
Felipe Contreras

Re: [PATCH v2 00/16] Make Gnome Credential helper more Gnome-y and support ancient distros

[Jeff King]

On Mon, Sep 23, 2013 at 11:49:01AM -0700, Brandon Casey wrote:

> Brandon Casey (16):
>   contrib/git-credential-gnome-keyring.c: remove unnecessary
>     pre-declarations
> [...]

Thanks, I think this is a good change. There were patches[1] from
Philipp about a year ago that went in the opposite direction, factoring
out common credential functions to be used by several helpers. We ended
up not merging them, because porting the wincred helper to the generic
implementation introduced complications.

And that was part of the reason for splitting out the helpers in the
first place: to let them worry about their own individual
system-specific quirks (and take advantage of system-specific helpers).
IOW, to make the gnome helper more gnome-y than git-y. And your patches
go in that direction, which I think is sane.

[1] http://thread.gmane.org/gmane.comp.version-control.git/204154/focus=204157

>   contrib/git-credential-gnome-keyring.c: report failure to store
>     password

Your subject lines are rather on the long side. I wonder if they would
be more readable as just:

  gnome-keyring: report failure to store password

and so forth. Anyone familiar with git.git knows that the "contrib"
and "git-credential-" bits are implied by "gnome-keyring". But it's not
a huge deal either way.

> Inserts a patch to fix the style issues for block statements.
> i.e. use "if ()" instead of "if()"

There are a ton of other style issues in the existing code (lack of
whitespace around operators and function arguments, and "char* foo"
instead of "char *foo" were the two I noticed). As a separate contrib/
project, I'm not that concerned, and I would say it is up to whoever is
nominally in charge of maintaining that contrib directory (Philipp or
John, in this case).

-Peff

Re: [PATCH v2 00/16] Make Gnome Credential helper more Gnome-y and support ancient distros

[Junio C Hamano]

This seems to post-date what Jonathan has kept in his 'pu'; is this
the latest (I have a huge backlog to wade through, so I'd rather
skip it if another reroll is coming and move on to other topics).

Thanks.

Re: [PATCH v2 00/16] Make Gnome Credential helper more Gnome-y and support ancient distros

[Brandon Casey]

On 10/15/2013 3:40 PM, Junio C Hamano wrote:
> This seems to post-date what Jonathan has kept in his 'pu'; is this
> the latest (I have a huge backlog to wade through, so I'd rather
> skip it if another reroll is coming and move on to other topics).
> 
> Thanks.

This is the latest.

I didn't have anything else planned.  I think Philipp planned to submit
some style cleanups on top for areas of the code I didn't touch.

-Brandon


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH v2 00/16] Make Gnome Credential helper more Gnome-y and support ancient distros

[Junio C Hamano]

Brandon Casey <bcasey@nvidia.com> writes:

> On 10/15/2013 3:40 PM, Junio C Hamano wrote:
>> This seems to post-date what Jonathan has kept in his 'pu'; is this
>> the latest (I have a huge backlog to wade through, so I'd rather
>> skip it if another reroll is coming and move on to other topics).
>> 
>> Thanks.
>
> This is the latest.
>
> I didn't have anything else planned.  I think Philipp planned to submit
> some style cleanups on top for areas of the code I didn't touch.

Thanks; will requeue.