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$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=john@szakmeister$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox