{"thread":{"id":"35463","subject":"[PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups","startedAt":"2013-12-03T10:32:44Z","lastAt":"2013-12-13T20:07:49Z","messageCount":8,"participants":["John Szakmeister","Junio C Hamano","Felipe Contreras"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"231422","messageId":"1386066764-49711-1-git-send-email-john@szakmeister.net","threadId":"35463","inReplyTo":null,"subject":"[PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups","fromName":"John Szakmeister","fromEmail":"john@szakmeister.net","sentAt":"2013-12-03T10:32:44Z","receivedAt":"2013-12-03T10:32:44Z","isPatch":true,"sender":{"key":"john@szakmeister.net","avatar":"https://avatars.githubusercontent.com/u/448087?v=4"},"body":"Signed-off-by: John Szakmeister <john@szakmeister.net>\n---\nThe gnome-keyring credential backend had a number of coding style\nviolations.  I believe this fixes all of them.\n\n .../gnome-keyring/git-credential-gnome-keyring.c   | 55 ++++++++++------------\n 1 file changed, 25 insertions(+), 30 deletions(-)\n\ndiff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\nindex 635c96b..1613404 100644\n--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\n+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\n@@ -95,9 +95,9 @@ static const char* gnome_keyring_result_to_message(GnomeKeyringResult result)\n \n static void gnome_keyring_done_cb(GnomeKeyringResult result, gpointer user_data)\n {\n-\tgpointer *data = (gpointer*) user_data;\n-\tint *done = (int*) data[0];\n-\tGnomeKeyringResult *r = (GnomeKeyringResult*) data[1];\n+\tgpointer *data = (gpointer *) user_data;\n+\tint *done = (int *) data[0];\n+\tGnomeKeyringResult *r = (GnomeKeyringResult *) data[1];\n \n \t*r = result;\n \t*done = 1;\n@@ -132,27 +132,25 @@ static GnomeKeyringResult gnome_keyring_item_delete_sync(const char *keyring, gu\n  */\n struct credential\n {\n-\tchar          *protocol;\n-\tchar          *host;\n+\tchar *protocol;\n+\tchar *host;\n \tunsigned short port;\n-\tchar          *path;\n-\tchar          *username;\n-\tchar          *password;\n+\tchar *path;\n+\tchar *username;\n+\tchar *password;\n };\n \n-#define CREDENTIAL_INIT \\\n-  { NULL,NULL,0,NULL,NULL,NULL }\n+#define CREDENTIAL_INIT { NULL, NULL, 0, NULL, NULL, NULL }\n \n-typedef int (*credential_op_cb)(struct credential*);\n+typedef int (*credential_op_cb)(struct credential *);\n \n struct credential_operation\n {\n-\tchar             *name;\n+\tchar *name;\n \tcredential_op_cb op;\n };\n \n-#define CREDENTIAL_OP_END \\\n-  { NULL,NULL }\n+#define CREDENTIAL_OP_END { NULL, NULL }\n \n /* ----------------- GNOME Keyring functions ----------------- */\n \n@@ -221,7 +219,7 @@ static int keyring_get(struct credential *c)\n static int keyring_store(struct credential *c)\n {\n \tguint32 item_id;\n-\tchar  *object = NULL;\n+\tchar *object = NULL;\n \tGnomeKeyringResult result;\n \n \t/*\n@@ -262,7 +260,7 @@ static int keyring_store(struct credential *c)\n \n static int keyring_erase(struct credential *c)\n {\n-\tchar  *object = NULL;\n+\tchar *object = NULL;\n \tGList *entries;\n \tGnomeKeyringNetworkPasswordData *password_data;\n \tGnomeKeyringResult result;\n@@ -298,8 +296,7 @@ static int keyring_erase(struct credential *c)\n \tif (result == GNOME_KEYRING_RESULT_CANCELLED)\n \t\treturn EXIT_SUCCESS;\n \n-\tif (result != GNOME_KEYRING_RESULT_OK)\n-\t{\n+\tif (result != GNOME_KEYRING_RESULT_OK) {\n \t\tg_critical(\"%s\", gnome_keyring_result_to_message(result));\n \t\treturn EXIT_FAILURE;\n \t}\n@@ -312,8 +309,7 @@ static int keyring_erase(struct credential *c)\n \n \tgnome_keyring_network_password_list_free(entries);\n \n-\tif (result != GNOME_KEYRING_RESULT_OK)\n-\t{\n+\tif (result != GNOME_KEYRING_RESULT_OK) {\n \t\tg_critical(\"%s\", gnome_keyring_result_to_message(result));\n \t\treturn EXIT_FAILURE;\n \t}\n@@ -325,9 +321,8 @@ static int keyring_erase(struct credential *c)\n  * Table with helper operation callbacks, used by generic\n  * credential helper main function.\n  */\n-static struct credential_operation const credential_helper_ops[] =\n-{\n-\t{ \"get\",   keyring_get   },\n+static struct credential_operation const credential_helper_ops[] = {\n+\t{ \"get\",   keyring_get },\n \t{ \"store\", keyring_store },\n \t{ \"erase\", keyring_erase },\n \tCREDENTIAL_OP_END\n@@ -370,7 +365,7 @@ static int credential_read(struct credential *c)\n \t\tif (!line_len)\n \t\t\tbreak;\n \n-\t\tvalue = strchr(buf,'=');\n+\t\tvalue = strchr(buf, '=');\n \t\tif (!value) {\n \t\t\tg_warning(\"invalid credential line: %s\", key);\n \t\t\tgnome_keyring_memory_free(buf);\n@@ -384,7 +379,7 @@ static int credential_read(struct credential *c)\n \t\t} else if (!strcmp(key, \"host\")) {\n \t\t\tg_free(c->host);\n \t\t\tc->host = g_strdup(value);\n-\t\t\tvalue = strrchr(c->host,':');\n+\t\t\tvalue = strrchr(c->host, ':');\n \t\t\tif (value) {\n \t\t\t\t*value++ = '\\0';\n \t\t\t\tc->port = atoi(value);\n@@ -429,16 +424,16 @@ static void credential_write(const struct credential *c)\n static void usage(const char *name)\n {\n \tstruct credential_operation const *try_op = credential_helper_ops;\n-\tconst char *basename = strrchr(name,'/');\n+\tconst char *basename = strrchr(name, '/');\n \n \tbasename = (basename) ? basename + 1 : name;\n \tfprintf(stderr, \"usage: %s <\", basename);\n \twhile (try_op->name) {\n-\t\tfprintf(stderr,\"%s\",(try_op++)->name);\n+\t\tfprintf(stderr, \"%s\", (try_op++)->name);\n \t\tif (try_op->name)\n-\t\t\tfprintf(stderr,\"%s\",\"|\");\n+\t\t\tfprintf(stderr, \"%s\", \"|\");\n \t}\n-\tfprintf(stderr,\"%s\",\">\\n\");\n+\tfprintf(stderr, \"%s\", \">\\n\");\n }\n \n int main(int argc, char *argv[])\n@@ -446,7 +441,7 @@ int main(int argc, char *argv[])\n \tint ret = EXIT_SUCCESS;\n \n \tstruct credential_operation const *try_op = credential_helper_ops;\n-\tstruct credential                  cred   = CREDENTIAL_INIT;\n+\tstruct credential cred = CREDENTIAL_INIT;\n \n \tif (!argv[1]) {\n \t\tusage(argv[0]);\n-- \n1.8.3.1\n"},{"id":"231558","messageId":"xmqqtxeoi24f.fsf@gitster.dls.corp.google.com","threadId":"35463","inReplyTo":"1386066764-49711-1-git-send-email-john@szakmeister.net","subject":"Re: [PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-05T00:16:00Z","receivedAt":"2013-12-05T00:16:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, will queue.\n"},{"id":"231695","messageId":"52a2ec51d8a67_29836d5e984f@nysa.notmuch","threadId":"35463","inReplyTo":"1386066764-49711-1-git-send-email-john@szakmeister.net","subject":"RE: [PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-12-07T09:37:21Z","receivedAt":"2013-12-07T09:37:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"John Szakmeister wrote:\n> Signed-off-by: John Szakmeister <john@szakmeister.net>\n> ---\n> The gnome-keyring credential backend had a number of coding style\n> violations.  I believe this fixes all of them.\n> \n>  .../gnome-keyring/git-credential-gnome-keyring.c   | 55 ++++++++++------------\n>  1 file changed, 25 insertions(+), 30 deletions(-)\n> \n> diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\n> index 635c96b..1613404 100644\n> --- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\n> +++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\n> @@ -95,9 +95,9 @@ static const char* gnome_keyring_result_to_message(GnomeKeyringResult result)\n>  \n>  static void gnome_keyring_done_cb(GnomeKeyringResult result, gpointer user_data)\n>  {\n> -\tgpointer *data = (gpointer*) user_data;\n> -\tint *done = (int*) data[0];\n> -\tGnomeKeyringResult *r = (GnomeKeyringResult*) data[1];\n> +\tgpointer *data = (gpointer *) user_data;\n> +\tint *done = (int *) data[0];\n> +\tGnomeKeyringResult *r = (GnomeKeyringResult *) data[1];\n\nNo need for these castings, a gpointer is void *, so there's no need to cast them.\n\n-- \nFelipe Contreras\n"},{"id":"231699","messageId":"1386412944-35106-1-git-send-email-john@szakmeister.net","threadId":"35463","inReplyTo":"1386066764-49711-1-git-send-email-john@szakmeister.net","subject":"[PATCH v2] contrib/git-credential-gnome-keyring.c: small stylistic cleanups","fromName":"John Szakmeister","fromEmail":"john@szakmeister.net","sentAt":"2013-12-07T10:42:24Z","receivedAt":"2013-12-07T10:42:24Z","isPatch":true,"sender":{"key":"john@szakmeister.net","avatar":"https://avatars.githubusercontent.com/u/448087?v=4"},"body":"Signed-off-by: John Szakmeister <john@szakmeister.net>\nReviewed-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\nv2 removes some unnecessary casting noticed by Felipe.  Thanks for the\nfeedback Felipe!\n\n .../gnome-keyring/git-credential-gnome-keyring.c   | 55 ++++++++++------------\n 1 file changed, 25 insertions(+), 30 deletions(-)\n\ndiff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\nindex 635c96b..302122c 100644\n--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\n+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\n@@ -95,9 +95,9 @@ static const char* gnome_keyring_result_to_message(GnomeKeyringResult result)\n \n static void gnome_keyring_done_cb(GnomeKeyringResult result, gpointer user_data)\n {\n-\tgpointer *data = (gpointer*) user_data;\n-\tint *done = (int*) data[0];\n-\tGnomeKeyringResult *r = (GnomeKeyringResult*) data[1];\n+\tgpointer *data = user_data;\n+\tint *done = data[0];\n+\tGnomeKeyringResult *r = data[1];\n \n \t*r = result;\n \t*done = 1;\n@@ -132,27 +132,25 @@ static GnomeKeyringResult gnome_keyring_item_delete_sync(const char *keyring, gu\n  */\n struct credential\n {\n-\tchar          *protocol;\n-\tchar          *host;\n+\tchar *protocol;\n+\tchar *host;\n \tunsigned short port;\n-\tchar          *path;\n-\tchar          *username;\n-\tchar          *password;\n+\tchar *path;\n+\tchar *username;\n+\tchar *password;\n };\n \n-#define CREDENTIAL_INIT \\\n-  { NULL,NULL,0,NULL,NULL,NULL }\n+#define CREDENTIAL_INIT { NULL, NULL, 0, NULL, NULL, NULL }\n \n-typedef int (*credential_op_cb)(struct credential*);\n+typedef int (*credential_op_cb)(struct credential *);\n \n struct credential_operation\n {\n-\tchar             *name;\n+\tchar *name;\n \tcredential_op_cb op;\n };\n \n-#define CREDENTIAL_OP_END \\\n-  { NULL,NULL }\n+#define CREDENTIAL_OP_END { NULL, NULL }\n \n /* ----------------- GNOME Keyring functions ----------------- */\n \n@@ -221,7 +219,7 @@ static int keyring_get(struct credential *c)\n static int keyring_store(struct credential *c)\n {\n \tguint32 item_id;\n-\tchar  *object = NULL;\n+\tchar *object = NULL;\n \tGnomeKeyringResult result;\n \n \t/*\n@@ -262,7 +260,7 @@ static int keyring_store(struct credential *c)\n \n static int keyring_erase(struct credential *c)\n {\n-\tchar  *object = NULL;\n+\tchar *object = NULL;\n \tGList *entries;\n \tGnomeKeyringNetworkPasswordData *password_data;\n \tGnomeKeyringResult result;\n@@ -298,8 +296,7 @@ static int keyring_erase(struct credential *c)\n \tif (result == GNOME_KEYRING_RESULT_CANCELLED)\n \t\treturn EXIT_SUCCESS;\n \n-\tif (result != GNOME_KEYRING_RESULT_OK)\n-\t{\n+\tif (result != GNOME_KEYRING_RESULT_OK) {\n \t\tg_critical(\"%s\", gnome_keyring_result_to_message(result));\n \t\treturn EXIT_FAILURE;\n \t}\n@@ -312,8 +309,7 @@ static int keyring_erase(struct credential *c)\n \n \tgnome_keyring_network_password_list_free(entries);\n \n-\tif (result != GNOME_KEYRING_RESULT_OK)\n-\t{\n+\tif (result != GNOME_KEYRING_RESULT_OK) {\n \t\tg_critical(\"%s\", gnome_keyring_result_to_message(result));\n \t\treturn EXIT_FAILURE;\n \t}\n@@ -325,9 +321,8 @@ static int keyring_erase(struct credential *c)\n  * Table with helper operation callbacks, used by generic\n  * credential helper main function.\n  */\n-static struct credential_operation const credential_helper_ops[] =\n-{\n-\t{ \"get\",   keyring_get   },\n+static struct credential_operation const credential_helper_ops[] = {\n+\t{ \"get\",   keyring_get },\n \t{ \"store\", keyring_store },\n \t{ \"erase\", keyring_erase },\n \tCREDENTIAL_OP_END\n@@ -370,7 +365,7 @@ static int credential_read(struct credential *c)\n \t\tif (!line_len)\n \t\t\tbreak;\n \n-\t\tvalue = strchr(buf,'=');\n+\t\tvalue = strchr(buf, '=');\n \t\tif (!value) {\n \t\t\tg_warning(\"invalid credential line: %s\", key);\n \t\t\tgnome_keyring_memory_free(buf);\n@@ -384,7 +379,7 @@ static int credential_read(struct credential *c)\n \t\t} else if (!strcmp(key, \"host\")) {\n \t\t\tg_free(c->host);\n \t\t\tc->host = g_strdup(value);\n-\t\t\tvalue = strrchr(c->host,':');\n+\t\t\tvalue = strrchr(c->host, ':');\n \t\t\tif (value) {\n \t\t\t\t*value++ = '\\0';\n \t\t\t\tc->port = atoi(value);\n@@ -429,16 +424,16 @@ static void credential_write(const struct credential *c)\n static void usage(const char *name)\n {\n \tstruct credential_operation const *try_op = credential_helper_ops;\n-\tconst char *basename = strrchr(name,'/');\n+\tconst char *basename = strrchr(name, '/');\n \n \tbasename = (basename) ? basename + 1 : name;\n \tfprintf(stderr, \"usage: %s <\", basename);\n \twhile (try_op->name) {\n-\t\tfprintf(stderr,\"%s\",(try_op++)->name);\n+\t\tfprintf(stderr, \"%s\", (try_op++)->name);\n \t\tif (try_op->name)\n-\t\t\tfprintf(stderr,\"%s\",\"|\");\n+\t\t\tfprintf(stderr, \"%s\", \"|\");\n \t}\n-\tfprintf(stderr,\"%s\",\">\\n\");\n+\tfprintf(stderr, \"%s\", \">\\n\");\n }\n \n int main(int argc, char *argv[])\n@@ -446,7 +441,7 @@ int main(int argc, char *argv[])\n \tint ret = EXIT_SUCCESS;\n \n \tstruct credential_operation const *try_op = credential_helper_ops;\n-\tstruct credential                  cred   = CREDENTIAL_INIT;\n+\tstruct credential cred = CREDENTIAL_INIT;\n \n \tif (!argv[1]) {\n \t\tusage(argv[0]);\n-- \n1.8.3.1\n"},{"id":"231794","messageId":"xmqq38m1292g.fsf@gitster.dls.corp.google.com","threadId":"35463","inReplyTo":"1386066764-49711-1-git-send-email-john@szakmeister.net","subject":"Re: [PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-09T18:06:15Z","receivedAt":"2013-12-09T18:06:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Szakmeister <john@szakmeister.net> writes:\n\n> Signed-off-by: John Szakmeister <john@szakmeister.net>\n> ---\n> The gnome-keyring credential backend had a number of coding style\n> violations.  I believe this fixes all of them.\n>\n>  .../gnome-keyring/git-credential-gnome-keyring.c   | 55 ++++++++++------------\n>  1 file changed, 25 insertions(+), 30 deletions(-)\n>\n> diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\n> index 635c96b..1613404 100644\n> --- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\n> +++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\n> @@ -95,9 +95,9 @@ static const char* gnome_keyring_result_to_message(GnomeKeyringResult result)\n>  \n>  static void gnome_keyring_done_cb(GnomeKeyringResult result, gpointer user_data)\n>  {\n> -\tgpointer *data = (gpointer*) user_data;\n> -\tint *done = (int*) data[0];\n> -\tGnomeKeyringResult *r = (GnomeKeyringResult*) data[1];\n> +\tgpointer *data = (gpointer *) user_data;\n> +\tint *done = (int *) data[0];\n> +\tGnomeKeyringResult *r = (GnomeKeyringResult *) data[1];\n\nI thought we cast without SP after the (typename), i.e.\n\n\tgpointer *data = (gpointer *)user_data;\n\nIt could be argued that a cast that turns a \"void *\" to a pointer to\nanother type can go, as Felipe noted, but I think that is better\ndone in a separate patch, perhaps as a follow-up to this \"small\nstylistic clean-ups\".\n\nI said \"it could be argued\" above, because I am on the fence on that\nchange.  If this were not using a type \"gpointer\", whose point is to\nhide what the actual implementation of that type is, but a plain\nvanilla \"void *\", then I would not have any doubt.  But it feels\nwrong to look behind that deliberate \"gpointer\" abstraction and take\nadvantage of the knowledge that it happens to be implemented as\n\"void *\" (and if we do not start from that knowledge, losing the\ncast is a wrong change).\n\nSo...\n"},{"id":"231854","messageId":"CAEBDL5Wb+fU=qX4eypt3SrAKQjWRqf9KkNDoutbt7fTsdk02rw@mail.gmail.com","threadId":"35463","inReplyTo":"xmqq38m1292g.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups","fromName":"John Szakmeister","fromEmail":"john@szakmeister.net","sentAt":"2013-12-10T11:13:32Z","receivedAt":"2013-12-10T11:13:32Z","isPatch":true,"sender":{"key":"john@szakmeister.net","avatar":"https://avatars.githubusercontent.com/u/448087?v=4"},"body":"On Mon, Dec 9, 2013 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:\n[snip]\n>\n> I thought we cast without SP after the (typename), i.e.\n>\n>         gpointer *data = (gpointer *)user_data;\n\nI've found a mixture of both in the code base, and the\nCodingGuidelines doesn't say either way.  I'm happy to switch the file\nto no SP after the typename if that's the project preference.\n\n> It could be argued that a cast that turns a \"void *\" to a pointer to\n> another type can go, as Felipe noted, but I think that is better\n> done in a separate patch, perhaps as a follow-up to this \"small\n> stylistic clean-ups\".\n>\n> I said \"it could be argued\" above, because I am on the fence on that\n> change.  If this were not using a type \"gpointer\", whose point is to\n> hide what the actual implementation of that type is, but a plain\n> vanilla \"void *\", then I would not have any doubt.  But it feels\n> wrong to look behind that deliberate \"gpointer\" abstraction and take\n> advantage of the knowledge that it happens to be implemented as\n> \"void *\" (and if we do not start from that knowledge, losing the\n> cast is a wrong change).\n\nTo be honest, I'm on the fence myself.  Let's just leave the original\npatch queued, and if the no SP is preferable, I can do that as a\nseparate patch.\n\n-John\n\nPS  Sorry about the repeat message Junio.  I forgot to CC the list.\n"},{"id":"231967","messageId":"xmqq8uvpskld.fsf@gitster.dls.corp.google.com","threadId":"35463","inReplyTo":"CAEBDL5Wb+fU=qX4eypt3SrAKQjWRqf9KkNDoutbt7fTsdk02rw@mail.gmail.com","subject":"Re: [PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-12T23:41:50Z","receivedAt":"2013-12-12T23:41:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Szakmeister <john@szakmeister.net> writes:\n\n> On Mon, Dec 9, 2013 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> [snip]\n>>\n>> I thought we cast without SP after the (typename), i.e.\n>>\n>>         gpointer *data = (gpointer *)user_data;\n>\n> I've found a mixture of both in the code base, and the\n> CodingGuidelines doesn't say either way.  I'm happy to switch the file\n> to no SP after the typename if that's the project preference.\n\nSomewhat arbitrary and unscientific, but between\n\n    git grep -e '[^f]([a-z_ ]* \\*)[^ ]' -- \\*.c | wc -l\n    422\n    $ git grep -e '[^f]([a-z_ ]* \\*) ' -- \\*.c | wc -l\n    233\n\nI see that we favor \"(struct blah *)apointer\" over \"(int *)\napointer\".  Many hits in the latter grep come from compat/\nthat are borrowed pieces of code we tend not to style-fix.\n\nThe leading [^f] is crudely excludes \"sizeof(typename *)\"; it does\nnot change the resulting picture in a major way, though.\n\nThanks.\n"},{"id":"231980","messageId":"xmqqeh5gqzu2.fsf@gitster.dls.corp.google.com","threadId":"35463","inReplyTo":"xmqq8uvpskld.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-13T20:07:49Z","receivedAt":"2013-12-13T20:07:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> John Szakmeister <john@szakmeister.net> writes:\n>\n>> On Mon, Dec 9, 2013 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> [snip]\n>>>\n>>> I thought we cast without SP after the (typename), i.e.\n>>>\n>>>         gpointer *data = (gpointer *)user_data;\n>>\n>> I've found a mixture of both in the code base, and the\n>> CodingGuidelines doesn't say either way.  I'm happy to switch the file\n>> to no SP after the typename if that's the project preference.\n>\n> Somewhat arbitrary and unscientific, but between\n>\n>     git grep -e '[^f]([a-z_ ]* \\*)[^ ]' -- \\*.c | wc -l\n>     422\n>     $ git grep -e '[^f]([a-z_ ]* \\*) ' -- \\*.c | wc -l\n>     233\n>\n> I see that we favor \"(struct blah *)apointer\" over \"(int *)\n> apointer\".  Many hits in the latter grep come from compat/\n> that are borrowed pieces of code we tend not to style-fix.\n>\n> The leading [^f] is crudely excludes \"sizeof(typename *)\"; it does\n> not change the resulting picture in a major way, though.\n>\n> Thanks.\n\nHere is a squashable diff on top of your clean-up:\n\n * A few more violations of the same \"asterisk sticks to what is the\n   pointer, not the name of the type\";\n\n * No SP between (typename) and castee;\n\n * Opening parenthesis of \"struct/union name\" comes on the same line\n   as the \"struct/union\" keyword;\n\n * Opening parenthesis of structured statements e.g. if/while/for/...\n   comes on the same line as the starting keyword;\n\n * Body of structured controls e.g. if/while/... on a separate line.\n\nI may have caught all of them, but I wasn't trying to be super\ncareful, so...\n\n\ndiff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\nindex 1613404..d45503c 100644\n--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\n+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c\n@@ -60,7 +60,7 @@\n #define gnome_keyring_memory_free gnome_keyring_free_password\n #define gnome_keyring_memory_strdup g_strdup\n \n-static const char* gnome_keyring_result_to_message(GnomeKeyringResult result)\n+static const char *gnome_keyring_result_to_message(GnomeKeyringResult result)\n {\n \tswitch (result) {\n \tcase GNOME_KEYRING_RESULT_OK:\n@@ -95,9 +95,9 @@ static const char* gnome_keyring_result_to_message(GnomeKeyringResult result)\n \n static void gnome_keyring_done_cb(GnomeKeyringResult result, gpointer user_data)\n {\n-\tgpointer *data = (gpointer *) user_data;\n-\tint *done = (int *) data[0];\n-\tGnomeKeyringResult *r = (GnomeKeyringResult *) data[1];\n+\tgpointer *data = (gpointer *)user_data;\n+\tint *done = (int *)data[0];\n+\tGnomeKeyringResult *r = (GnomeKeyringResult *)data[1];\n \n \t*r = result;\n \t*done = 1;\n@@ -130,8 +130,7 @@ static GnomeKeyringResult gnome_keyring_item_delete_sync(const char *keyring, gu\n /*\n  * This credential struct and API is simplified from git's credential.{h,c}\n  */\n-struct credential\n-{\n+struct credential {\n \tchar *protocol;\n \tchar *host;\n \tunsigned short port;\n@@ -144,8 +143,7 @@ struct credential\n \n typedef int (*credential_op_cb)(struct credential *);\n \n-struct credential_operation\n-{\n+struct credential_operation {\n \tchar *name;\n \tcredential_op_cb op;\n };\n@@ -155,7 +153,7 @@ struct credential_operation\n /* ----------------- GNOME Keyring functions ----------------- */\n \n /* create a special keyring option string, if path is given */\n-static char* keyring_object(struct credential *c)\n+static char *keyring_object(struct credential *c)\n {\n \tif (!c->path)\n \t\treturn NULL;\n@@ -168,7 +166,7 @@ static char* keyring_object(struct credential *c)\n \n static int keyring_get(struct credential *c)\n {\n-\tchar* object = NULL;\n+\tchar *object = NULL;\n \tGList *entries;\n \tGnomeKeyringNetworkPasswordData *password_data;\n \tGnomeKeyringResult result;\n@@ -202,7 +200,7 @@ static int keyring_get(struct credential *c)\n \t}\n \n \t/* pick the first one from the list */\n-\tpassword_data = (GnomeKeyringNetworkPasswordData *) entries->data;\n+\tpassword_data = (GnomeKeyringNetworkPasswordData *)entries->data;\n \n \tgnome_keyring_memory_free(c->password);\n \tc->password = gnome_keyring_memory_strdup(password_data->password);\n@@ -302,7 +300,7 @@ static int keyring_erase(struct credential *c)\n \t}\n \n \t/* pick the first one from the list (delete all matches?) */\n-\tpassword_data = (GnomeKeyringNetworkPasswordData *) entries->data;\n+\tpassword_data = (GnomeKeyringNetworkPasswordData *)entries->data;\n \n \tresult = gnome_keyring_item_delete_sync(\n \t\tpassword_data->keyring, password_data->item_id);\n@@ -355,12 +353,11 @@ static int credential_read(struct credential *c)\n \n \tkey = buf = gnome_keyring_memory_alloc(1024);\n \n-\twhile (fgets(buf, 1024, stdin))\n-\t{\n+\twhile (fgets(buf, 1024, stdin)) {\n \t\tline_len = strlen(buf);\n \n \t\tif (line_len && buf[line_len-1] == '\\n')\n-\t\t\tbuf[--line_len]='\\0';\n+\t\t\tbuf[--line_len] = '\\0';\n \n \t\tif (!line_len)\n \t\t\tbreak;\n@@ -393,7 +390,8 @@ static int credential_read(struct credential *c)\n \t\t} else if (!strcmp(key, \"password\")) {\n \t\t\tgnome_keyring_memory_free(c->password);\n \t\t\tc->password = gnome_keyring_memory_strdup(value);\n-\t\t\twhile (*value) *value++ = '\\0';\n+\t\t\twhile (*value)\n+\t\t\t\t*value++ = '\\0';\n \t\t}\n \t\t/*\n \t\t * Ignore other lines; we don't know what they mean, but\n-- \n1.8.5.1-251-gaaad5e7\n"}]}