From: Junio C Hamano <gitster@pobox.com>
To: John Szakmeister <john@szakmeister.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups
Date: Fri, 13 Dec 2013 12:07:49 -0800 [thread overview]
Message-ID: <xmqqeh5gqzu2.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqq8uvpskld.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Thu, 12 Dec 2013 15:41:50 -0800")
Junio C Hamano <gitster@pobox.com> writes:
> John Szakmeister <john@szakmeister.net> writes:
>
>> On Mon, Dec 9, 2013 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> [snip]
>>>
>>> I thought we cast without SP after the (typename), i.e.
>>>
>>> gpointer *data = (gpointer *)user_data;
>>
>> I've found a mixture of both in the code base, and the
>> CodingGuidelines doesn't say either way. I'm happy to switch the file
>> to no SP after the typename if that's the project preference.
>
> Somewhat arbitrary and unscientific, but between
>
> git grep -e '[^f]([a-z_ ]* \*)[^ ]' -- \*.c | wc -l
> 422
> $ git grep -e '[^f]([a-z_ ]* \*) ' -- \*.c | wc -l
> 233
>
> I see that we favor "(struct blah *)apointer" over "(int *)
> apointer". Many hits in the latter grep come from compat/
> that are borrowed pieces of code we tend not to style-fix.
>
> The leading [^f] is crudely excludes "sizeof(typename *)"; it does
> not change the resulting picture in a major way, though.
>
> Thanks.
Here is a squashable diff on top of your clean-up:
* A few more violations of the same "asterisk sticks to what is the
pointer, not the name of the type";
* No SP between (typename) and castee;
* Opening parenthesis of "struct/union name" comes on the same line
as the "struct/union" keyword;
* Opening parenthesis of structured statements e.g. if/while/for/...
comes on the same line as the starting keyword;
* Body of structured controls e.g. if/while/... on a separate line.
I may have caught all of them, but I wasn't trying to be super
careful, so...
diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 1613404..d45503c 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -60,7 +60,7 @@
#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)
+static const char *gnome_keyring_result_to_message(GnomeKeyringResult result)
{
switch (result) {
case GNOME_KEYRING_RESULT_OK:
@@ -95,9 +95,9 @@ static const char* gnome_keyring_result_to_message(GnomeKeyringResult result)
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];
+ gpointer *data = (gpointer *)user_data;
+ int *done = (int *)data[0];
+ GnomeKeyringResult *r = (GnomeKeyringResult *)data[1];
*r = result;
*done = 1;
@@ -130,8 +130,7 @@ static GnomeKeyringResult gnome_keyring_item_delete_sync(const char *keyring, gu
/*
* This credential struct and API is simplified from git's credential.{h,c}
*/
-struct credential
-{
+struct credential {
char *protocol;
char *host;
unsigned short port;
@@ -144,8 +143,7 @@ struct credential
typedef int (*credential_op_cb)(struct credential *);
-struct credential_operation
-{
+struct credential_operation {
char *name;
credential_op_cb op;
};
@@ -155,7 +153,7 @@ struct credential_operation
/* ----------------- GNOME Keyring functions ----------------- */
/* create a special keyring option string, if path is given */
-static char* keyring_object(struct credential *c)
+static char *keyring_object(struct credential *c)
{
if (!c->path)
return NULL;
@@ -168,7 +166,7 @@ static char* keyring_object(struct credential *c)
static int keyring_get(struct credential *c)
{
- char* object = NULL;
+ char *object = NULL;
GList *entries;
GnomeKeyringNetworkPasswordData *password_data;
GnomeKeyringResult result;
@@ -202,7 +200,7 @@ static int keyring_get(struct credential *c)
}
/* pick the first one from the list */
- password_data = (GnomeKeyringNetworkPasswordData *) entries->data;
+ password_data = (GnomeKeyringNetworkPasswordData *)entries->data;
gnome_keyring_memory_free(c->password);
c->password = gnome_keyring_memory_strdup(password_data->password);
@@ -302,7 +300,7 @@ static int keyring_erase(struct credential *c)
}
/* pick the first one from the list (delete all matches?) */
- password_data = (GnomeKeyringNetworkPasswordData *) entries->data;
+ password_data = (GnomeKeyringNetworkPasswordData *)entries->data;
result = gnome_keyring_item_delete_sync(
password_data->keyring, password_data->item_id);
@@ -355,12 +353,11 @@ static int credential_read(struct credential *c)
key = buf = gnome_keyring_memory_alloc(1024);
- while (fgets(buf, 1024, stdin))
- {
+ while (fgets(buf, 1024, stdin)) {
line_len = strlen(buf);
if (line_len && buf[line_len-1] == '\n')
- buf[--line_len]='\0';
+ buf[--line_len] = '\0';
if (!line_len)
break;
@@ -393,7 +390,8 @@ static int credential_read(struct credential *c)
} else if (!strcmp(key, "password")) {
gnome_keyring_memory_free(c->password);
c->password = gnome_keyring_memory_strdup(value);
- while (*value) *value++ = '\0';
+ while (*value)
+ *value++ = '\0';
}
/*
* Ignore other lines; we don't know what they mean, but
--
1.8.5.1-251-gaaad5e7
prev parent reply other threads:[~2013-12-13 20:07 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-03 10:32 [PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups John Szakmeister
2013-12-05 0:16 ` Junio C Hamano
2013-12-07 9:37 ` Felipe Contreras
2013-12-07 10:42 ` [PATCH v2] " John Szakmeister
2013-12-09 18:06 ` [PATCH] " Junio C Hamano
2013-12-10 11:13 ` John Szakmeister
2013-12-12 23:41 ` Junio C Hamano
2013-12-13 20:07 ` Junio C Hamano [this message]
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=xmqqeh5gqzu2.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=john@szakmeister.net \
/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.