* [PATCH 01/15] contrib/git-credential-gnome-keyring.c: remove unnecessary pre-declarations
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
@ 2013-09-23 5:07 ` Brandon Casey
2013-09-23 5:07 ` [PATCH 02/15] contrib/git-credential-gnome-keyring.c: remove unused die() function Brandon Casey
` (14 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:07 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 02/15] contrib/git-credential-gnome-keyring.c: remove unused die() function
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
2013-09-23 5:07 ` [PATCH 01/15] contrib/git-credential-gnome-keyring.c: remove unnecessary pre-declarations Brandon Casey
@ 2013-09-23 5:07 ` Brandon Casey
2013-09-23 5:07 ` [PATCH 03/15] contrib/git-credential-gnome-keyring.c: add static where applicable Brandon Casey
` (13 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:07 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 03/15] contrib/git-credential-gnome-keyring.c: add static where applicable
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
2013-09-23 5:07 ` [PATCH 01/15] contrib/git-credential-gnome-keyring.c: remove unnecessary pre-declarations Brandon Casey
2013-09-23 5:07 ` [PATCH 02/15] contrib/git-credential-gnome-keyring.c: remove unused die() function Brandon Casey
@ 2013-09-23 5:07 ` Brandon Casey
2013-09-23 5:08 ` [PATCH 04/15] contrib/git-credential-gnome-keyring.c: exit non-zero when called incorrectly Brandon Casey
` (12 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:07 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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 4334f23..ad23dbf 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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 04/15] contrib/git-credential-gnome-keyring.c: exit non-zero when called incorrectly
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
` (2 preceding siblings ...)
2013-09-23 5:07 ` [PATCH 03/15] contrib/git-credential-gnome-keyring.c: add static where applicable Brandon Casey
@ 2013-09-23 5:08 ` Brandon Casey
2013-09-23 5:08 ` [PATCH 05/15] contrib/git-credential-gnome-keyring.c: set Gnome application name Brandon Casey
` (11 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:08 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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 ad23dbf..5459ba7 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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 05/15] contrib/git-credential-gnome-keyring.c: set Gnome application name
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
` (3 preceding siblings ...)
2013-09-23 5:08 ` [PATCH 04/15] contrib/git-credential-gnome-keyring.c: exit non-zero when called incorrectly Brandon Casey
@ 2013-09-23 5:08 ` Brandon Casey
2013-09-23 5:08 ` [PATCH 06/15] contrib/git-credential-gnome-keyring.c: strlen() returns size_t, not ssize_t Brandon Casey
` (10 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:08 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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 5459ba7..5779770 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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 06/15] contrib/git-credential-gnome-keyring.c: strlen() returns size_t, not ssize_t
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
` (4 preceding siblings ...)
2013-09-23 5:08 ` [PATCH 05/15] contrib/git-credential-gnome-keyring.c: set Gnome application name Brandon Casey
@ 2013-09-23 5:08 ` Brandon Casey
2013-09-23 5:08 ` [PATCH 07/15] contrib/git-credential-gnome-keyring.c: ensure buffer is non-empty before accessing Brandon Casey
` (9 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:08 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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 5779770..1081224 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -307,7 +307,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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 07/15] contrib/git-credential-gnome-keyring.c: ensure buffer is non-empty before accessing
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
` (5 preceding siblings ...)
2013-09-23 5:08 ` [PATCH 06/15] contrib/git-credential-gnome-keyring.c: strlen() returns size_t, not ssize_t Brandon Casey
@ 2013-09-23 5:08 ` Brandon Casey
2013-09-23 5:43 ` Felipe Contreras
2013-09-23 5:08 ` [PATCH 08/15] contrib/git-credential-gnome-keyring.c: use gnome helpers in keyring_object() Brandon Casey
` (8 subsequent siblings)
15 siblings, 1 reply; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:08 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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 1081224..8ae2eab 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -315,7 +315,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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* RE: [PATCH 07/15] contrib/git-credential-gnome-keyring.c: ensure buffer is non-empty before accessing
2013-09-23 5:08 ` [PATCH 07/15] contrib/git-credential-gnome-keyring.c: ensure buffer is non-empty before accessing Brandon Casey
@ 2013-09-23 5:43 ` Felipe Contreras
2013-09-23 17:21 ` Brandon Casey
0 siblings, 1 reply; 20+ messages in thread
From: Felipe Contreras @ 2013-09-23 5:43 UTC (permalink / raw)
To: Brandon Casey, git; +Cc: pah, Brandon Casey
Brandon Casey wrote:
> 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 1081224..8ae2eab 100644
> --- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
> +++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
> @@ -315,7 +315,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')
The style is if ().
--
Felipe Contreras
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 07/15] contrib/git-credential-gnome-keyring.c: ensure buffer is non-empty before accessing
2013-09-23 5:43 ` Felipe Contreras
@ 2013-09-23 17:21 ` Brandon Casey
0 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 17:21 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git@vger.kernel.org, pah
Thanks.
On Sun, Sep 22, 2013 at 10:43 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Brandon Casey wrote:
>> 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 1081224..8ae2eab 100644
>> --- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
>> +++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
>> @@ -315,7 +315,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')
>
> The style is if ().
>
> --
> Felipe Contreras
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 08/15] contrib/git-credential-gnome-keyring.c: use gnome helpers in keyring_object()
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
` (6 preceding siblings ...)
2013-09-23 5:08 ` [PATCH 07/15] contrib/git-credential-gnome-keyring.c: ensure buffer is non-empty before accessing Brandon Casey
@ 2013-09-23 5:08 ` Brandon Casey
2013-09-23 5:08 ` [PATCH 09/15] contrib/git-credential-gnome-keyring.c: use secure memory functions for passwds Brandon Casey
` (7 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:08 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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 8ae2eab..7565765 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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 09/15] contrib/git-credential-gnome-keyring.c: use secure memory functions for passwds
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
` (7 preceding siblings ...)
2013-09-23 5:08 ` [PATCH 08/15] contrib/git-credential-gnome-keyring.c: use gnome helpers in keyring_object() Brandon Casey
@ 2013-09-23 5:08 ` Brandon Casey
2013-09-23 5:08 ` [PATCH 10/15] contrib/git-credential-gnome-keyring.c: use secure memory for reading passwords Brandon Casey
` (6 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:08 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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 7565765..ff2f48c 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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 10/15] contrib/git-credential-gnome-keyring.c: use secure memory for reading passwords
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
` (8 preceding siblings ...)
2013-09-23 5:08 ` [PATCH 09/15] contrib/git-credential-gnome-keyring.c: use secure memory functions for passwds Brandon Casey
@ 2013-09-23 5:08 ` Brandon Casey
2013-09-23 5:08 ` [PATCH 11/15] contrib/git-credential-gnome-keyring.c: use glib memory allocation functions Brandon Casey
` (5 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:08 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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 ff2f48c..94a65b2 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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 11/15] contrib/git-credential-gnome-keyring.c: use glib memory allocation functions
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
` (9 preceding siblings ...)
2013-09-23 5:08 ` [PATCH 10/15] contrib/git-credential-gnome-keyring.c: use secure memory for reading passwords Brandon Casey
@ 2013-09-23 5:08 ` Brandon Casey
2013-09-23 5:08 ` [PATCH 12/15] contrib/git-credential-gnome-keyring.c: use glib messaging functions Brandon Casey
` (4 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:08 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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 94a65b2..5b10e3e 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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 12/15] contrib/git-credential-gnome-keyring.c: use glib messaging functions
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
` (10 preceding siblings ...)
2013-09-23 5:08 ` [PATCH 11/15] contrib/git-credential-gnome-keyring.c: use glib memory allocation functions Brandon Casey
@ 2013-09-23 5:08 ` Brandon Casey
2013-09-23 5:08 ` [PATCH 13/15] contrib/git-credential-gnome-keyring.c: report failure to store password Brandon Casey
` (3 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:08 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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 5b10e3e..a6369a3 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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 13/15] contrib/git-credential-gnome-keyring.c: report failure to store password
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
` (11 preceding siblings ...)
2013-09-23 5:08 ` [PATCH 12/15] contrib/git-credential-gnome-keyring.c: use glib messaging functions Brandon Casey
@ 2013-09-23 5:08 ` Brandon Casey
2013-09-23 5:08 ` [PATCH 14/15] contrib/git-credential-gnome-keyring.c: support ancient gnome-keyring Brandon Casey
` (2 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:08 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
Produce an error message when we fail to store a password to the keyring.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
contrib/credential/gnome-keyring/git-credential-gnome-keyring.c | 9 ++++++++-
1 file changed, 8 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 a6369a3..6fa1278 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,12 @@ static int keyring_store(struct credential *c)
&item_id);
g_free(object);
+
+ if (result != GNOME_KEYRING_RESULT_OK) {
+ g_critical("%s", gnome_keyring_result_to_message(result));
+ return EXIT_FAILURE;
+ }
+
return EXIT_SUCCESS;
}
--
1.8.4.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 14/15] contrib/git-credential-gnome-keyring.c: support ancient gnome-keyring
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
` (12 preceding siblings ...)
2013-09-23 5:08 ` [PATCH 13/15] contrib/git-credential-gnome-keyring.c: report failure to store password Brandon Casey
@ 2013-09-23 5:08 ` Brandon Casey
2013-09-23 5:08 ` [PATCH 15/15] contrib/git-credential-gnome-keyring.c: support really " Brandon Casey
2013-09-23 10:20 ` [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros John Szakmeister
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:08 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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 6fa1278..f8f4df9 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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 15/15] contrib/git-credential-gnome-keyring.c: support really ancient gnome-keyring
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
` (13 preceding siblings ...)
2013-09-23 5:08 ` [PATCH 14/15] contrib/git-credential-gnome-keyring.c: support ancient gnome-keyring Brandon Casey
@ 2013-09-23 5:08 ` Brandon Casey
2013-09-23 10:20 ` [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros John Szakmeister
15 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 5:08 UTC (permalink / raw)
To: git; +Cc: pah, Brandon Casey
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>
---
.../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 f8f4df9..ce2ddee 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)
}
}
+/*
+ * Just a guess to support RHEL 4.X.
+ * 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.489.g545bc72
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros
2013-09-23 5:07 [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
` (14 preceding siblings ...)
2013-09-23 5:08 ` [PATCH 15/15] contrib/git-credential-gnome-keyring.c: support really " Brandon Casey
@ 2013-09-23 10:20 ` John Szakmeister
2013-09-23 17:21 ` Brandon Casey
15 siblings, 1 reply; 20+ messages in thread
From: John Szakmeister @ 2013-09-23 10:20 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, pah
On Sun, Sep 22, 2013 at 10:07:56PM -0700, Brandon Casey wrote:
> A few cleanups, followed by improved usage of the glib library (no need
> to reinvent the wheel when glib provides the necessary functionality), and
> then the addition of support for RHEL 4.x and 5.x.
>
> Brandon Casey (15):
> 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: add static where applicable
> contrib/git-credential-gnome-keyring.c: exit non-zero when called
> incorrectly
> contrib/git-credential-gnome-keyring.c: set Gnome application name
> 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: 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
I reviewed all of the commits in this series, and most are nice
cleanups. The only thing I'm a little shaky on is RHEL4
support (patch 15). In particular, this:
+/*
+ * Just a guess to support RHEL 4.X.
+ * Glib 2.8 was roughly Gnome 2.12 ?
+ * Which was released with gnome-keyring 0.4.3 ??
+ */
Has that code been tested on RHEL4 at all? I imagine it's hard
to come by--it's pretty darn old--but it feels like a mistake to
commit untested code.
Otherwise, there are a few stylistic nits that I'd like to clean
up. Only one was introduced by you--Felipe pointed it out--and
I have a patch for the rest that I can submit after your series
has been applied.
Acked-by: John Szakmeister <john@szakmeister.net>
-John
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros
2013-09-23 10:20 ` [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros John Szakmeister
@ 2013-09-23 17:21 ` Brandon Casey
0 siblings, 0 replies; 20+ messages in thread
From: Brandon Casey @ 2013-09-23 17:21 UTC (permalink / raw)
To: John Szakmeister; +Cc: git@vger.kernel.org, pah
On Mon, Sep 23, 2013 at 3:20 AM, John Szakmeister <john@szakmeister.net> wrote:
> On Sun, Sep 22, 2013 at 10:07:56PM -0700, Brandon Casey wrote:
>> A few cleanups, followed by improved usage of the glib library (no need
>> to reinvent the wheel when glib provides the necessary functionality), and
>> then the addition of support for RHEL 4.x and 5.x.
>>
>> Brandon Casey (15):
<snip>
> I reviewed all of the commits in this series, and most are nice
> cleanups. The only thing I'm a little shaky on is RHEL4
> support (patch 15). In particular, this:
>
> +/*
> + * Just a guess to support RHEL 4.X.
> + * Glib 2.8 was roughly Gnome 2.12 ?
> + * Which was released with gnome-keyring 0.4.3 ??
> + */
>
> Has that code been tested on RHEL4 at all? I imagine it's hard
> to come by--it's pretty darn old--but it feels like a mistake to
> commit untested code.
The (poorly worded) comment is referring to the version of glib that
is being tested by the pre-processor statements. Since gnome-keyring
doesn't provide a way to test its version, and I'm not sure exactly
which gnome release included gnome-keyring 0.4.3 or which glib version
was distributed with which gnome version, I'm just roughly guessing
based on dates and not-conclusive google searches that 2.8 is
reasonable.
I'll try to clarify the wording here.
The code has been tested on CentOS 4.X.
> Otherwise, there are a few stylistic nits that I'd like to clean
> up. Only one was introduced by you--Felipe pointed it out--and
Well, not exactly "introduced" by me, but I guess I can fix it in that
same patch. :)
> I have a patch for the rest that I can submit after your series
> has been applied.
Sounds good.
> Acked-by: John Szakmeister <john@szakmeister.net>
-Brandon
^ permalink raw reply [flat|nested] 20+ messages in thread