Subject: Re: [PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups

From: Junio C Hamano <[email-masked]>
Date: Fri, 13 Dec 2013 12:07:49 -0800

Junio C Hamano <[email-masked]> writes:

> John Szakmeister <[email-masked]> writes:
>
>> On Mon, Dec 9, 2013 at 1:06 PM, Junio C Hamano <[email-masked]> 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