From: John Szakmeister <john@szakmeister.net>
To: Brandon Casey <drafnel@gmail.com>
Cc: git@vger.kernel.org, pah@qo.cx
Subject: Re: [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros
Date: Mon, 23 Sep 2013 06:20:22 -0400 [thread overview]
Message-ID: <20130923102022.GA96514@yoda.local> (raw)
In-Reply-To: <1379912891-12277-1-git-send-email-drafnel@gmail.com>
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
next prev parent reply other threads:[~2013-09-23 10:20 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` [PATCH 03/15] contrib/git-credential-gnome-keyring.c: add static where applicable Brandon Casey
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 ` [PATCH 05/15] contrib/git-credential-gnome-keyring.c: set Gnome application name 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
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
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 ` [PATCH 09/15] contrib/git-credential-gnome-keyring.c: use secure memory functions for passwds Brandon Casey
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 ` [PATCH 11/15] contrib/git-credential-gnome-keyring.c: use glib memory allocation functions Brandon Casey
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 ` [PATCH 13/15] contrib/git-credential-gnome-keyring.c: report failure to store password Brandon Casey
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 ` [PATCH 15/15] contrib/git-credential-gnome-keyring.c: support really " Brandon Casey
2013-09-23 10:20 ` John Szakmeister [this message]
2013-09-23 17:21 ` [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Brandon Casey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130923102022.GA96514@yoda.local \
--to=john@szakmeister.net \
--cc=drafnel@gmail.com \
--cc=git@vger.kernel.org \
--cc=pah@qo.cx \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.