{"thread":{"id":"38038","subject":"[PATCH 2/3 v2] mailmap: use higher level string list functions","startedAt":"2014-11-24T21:52:12Z","lastAt":"2014-12-01T02:33:14Z","messageCount":6,"participants":["Stefan Beller","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"252458","messageId":"1416865932-18285-1-git-send-email-sbeller@google.com","threadId":"38038","inReplyTo":"xmqq3898w9v4.fsf@gitster.dls.corp.google.com","subject":"[PATCH 2/3 v2] mailmap: use higher level string list functions","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-24T21:52:12Z","receivedAt":"2014-11-24T21:52:12Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"No functional changes intended. This commit makes user of higher level\nand better documented functions of the string list API, so the code is\nmore understandable.\n\nNote that also the required computational amount should not change\nin principal as we need to look up the item no matter if it is already\npart of the list or not. Once looked up, insertion comes for free.\n\nSigned-off-by: Stefan Beller \n---\n\nChanges since Version 1:\n* Remove declaration-after-statement.\n\n mailmap.c | 12 ++++--------\n 1 file changed, 4 insertions(+), 8 deletions(-)\n\ndiff --git a/mailmap.c b/mailmap.c\nindex 81890a6..3b00a65 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -71,6 +71,7 @@ static void add_mapping(struct string_list *map,\n \t\t\tchar *old_name, char *old_email)\n {\n \tstruct mailmap_entry *me;\n+\tstruct string_list_item *item;\n \tint index;\n \n \tif (old_email == NULL) {\n@@ -78,15 +79,10 @@ static void add_mapping(struct string_list *map,\n \t\tnew_email = NULL;\n \t}\n \n-\tif ((index = string_list_find_insert_index(map, old_email, 1)) < 0) {\n-\t\t/* mailmap entry exists, invert index value */\n-\t\tindex = -1 - index;\n-\t\tme = (struct mailmap_entry *)map->items[index].util;\n+\titem = string_list_insert(map, old_email);\n+\tif (item->util) {\n+\t\tme = (struct mailmap_entry *)item->util;\n \t} else {\n-\t\t/* create mailmap entry */\n-\t\tstruct string_list_item *item;\n-\n-\t\titem = string_list_insert_at_index(map, index, old_email);\n \t\tme = xcalloc(1, sizeof(struct mailmap_entry));\n \t\tme->namemap.strdup_strings = 1;\n \t\tme->namemap.cmp = namemap_cmp;\n-- \n2.2.0.rc3\n"},{"id":"252483","messageId":"CAPig+cQ_4A-0LOgXXG5qLeHOev+g8KMq5osKz34AFijGuyRidQ@mail.gmail.com","threadId":"38038","inReplyTo":"1416865932-18285-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH 2/3 v2] mailmap: use higher level string list functions","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-11-25T02:36:08Z","receivedAt":"2014-11-25T02:36:08Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Nov 24, 2014 at 4:52 PM, Stefan Beller wrote:\n> No functional changes intended. This commit makes user of higher level\n\ns/user/use/\n\n> and better documented functions of the string list API, so the code is\n> more understandable.\n>\n> Note that also the required computational amount should not change\n> in principal as we need to look up the item no matter if it is already\n> part of the list or not. Once looked up, insertion comes for free.\n>\n> Signed-off-by: Stefan Beller \n"},{"id":"252485","messageId":"1416887054-27204-1-git-send-email-sbeller@google.com","threadId":"38038","inReplyTo":"CAPig+cQ_4A-0LOgXXG5qLeHOev+g8KMq5osKz34AFijGuyRidQ@mail.gmail.com","subject":"[PATCHv3 2/3] mailmap: use higher level string list functions","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-25T03:44:14Z","receivedAt":"2014-11-25T03:44:14Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"No functional changes intended. This commit makes use of higher level\nand better documented functions of the string list API, so the code is\nmore understandable.\n\nNote that also the required computational amount should not change\nin principal as we need to look up the item no matter if it is already\npart of the list or not. Once looked up, insertion comes for free.\n\nSigned-off-by: Stefan Beller \n---\nChanges since Version 1:\n* Remove declaration-after-statement.\n\nChanges Version 1 to Version 2:\n* typo in commit message\n\n mailmap.c | 12 ++++--------\n 1 file changed, 4 insertions(+), 8 deletions(-)\n\ndiff --git a/mailmap.c b/mailmap.c\nindex 81890a6..3b00a65 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -71,6 +71,7 @@ static void add_mapping(struct string_list *map,\n \t\t\tchar *old_name, char *old_email)\n {\n \tstruct mailmap_entry *me;\n+\tstruct string_list_item *item;\n \tint index;\n \n \tif (old_email == NULL) {\n@@ -78,15 +79,10 @@ static void add_mapping(struct string_list *map,\n \t\tnew_email = NULL;\n \t}\n \n-\tif ((index = string_list_find_insert_index(map, old_email, 1)) < 0) {\n-\t\t/* mailmap entry exists, invert index value */\n-\t\tindex = -1 - index;\n-\t\tme = (struct mailmap_entry *)map->items[index].util;\n+\titem = string_list_insert(map, old_email);\n+\tif (item->util) {\n+\t\tme = (struct mailmap_entry *)item->util;\n \t} else {\n-\t\t/* create mailmap entry */\n-\t\tstruct string_list_item *item;\n-\n-\t\titem = string_list_insert_at_index(map, index, old_email);\n \t\tme = xcalloc(1, sizeof(struct mailmap_entry));\n \t\tme->namemap.strdup_strings = 1;\n \t\tme->namemap.cmp = namemap_cmp;\n-- \n2.2.0.rc3\n"},{"id":"252648","messageId":"CAPig+cRNuuDDBV0-TwANuiv+f_c1mfXp2Q4rF1Sj5mtJoLoqHQ@mail.gmail.com","threadId":"38038","inReplyTo":"CAO2U3QjNua2HvJKLnq80mPFEp931yLzHKENKo-LHm4CFZWRhBA@mail.gmail.com","subject":"Re: [PATCHv3 2/3] mailmap: use higher level string list functions","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-11-27T19:57:10Z","receivedAt":"2014-11-27T19:57:10Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Nov 27, 2014 at 1:44 PM, Michael Blume wrote:\n> The variable index seems to be unused/uninitialized now -- it's still\n> printed in debug messages, but if I'm reading correctly, its contents are\n> going to be nonsense.\n\nNice catch.\n\n(Aside: Please don't top-post on this list [1].)\n\n[1]: https://lkml.org/lkml/2005/1/11/111\n\n\n> On Mon Nov 24 2014 at 7:48:27 PM Stefan Beller wrote:\n>>\n>> No functional changes intended. This commit makes use of higher level\n>> and better documented functions of the string list API, so the code is\n>> more understandable.\n>>\n>> Note that also the required computational amount should not change\n>> in principal as we need to look up the item no matter if it is already\n>> part of the list or not. Once looked up, insertion comes for free.\n>>\n>> Signed-off-by: Stefan Beller \n>> ---\n>> Changes since Version 1:\n>> * Remove declaration-after-statement.\n>>\n>> Changes Version 1 to Version 2:\n>> * typo in commit message\n>>\n>> mailmap.c | 12 ++++--------\n>> 1 file changed, 4 insertions(+), 8 deletions(-)\n>>\n>> diff --git a/mailmap.c b/mailmap.c\n>> index 81890a6..3b00a65 100644\n>> --- a/mailmap.c\n>> +++ b/mailmap.c\n>> @@ -71,6 +71,7 @@ static void add_mapping(struct string_list *map,\n>> char *old_name, char *old_email)\n>> {\n>> struct mailmap_entry *me;\n>> + struct string_list_item *item;\n>> int index;\n>>\n>> if (old_email == NULL) {\n>> @@ -78,15 +79,10 @@ static void add_mapping(struct string_list *map,\n>> new_email = NULL;\n>> }\n>>\n>> - if ((index = string_list_find_insert_index(map, old_email, 1)) <\n>> 0) {\n>> - /* mailmap entry exists, invert index value */\n>> - index = -1 - index;\n>> - me = (struct mailmap_entry *)map->items[index].util;\n>> + item = string_list_insert(map, old_email);\n>> + if (item->util) {\n>> + me = (struct mailmap_entry *)item->util;\n>> } else {\n>> - /* create mailmap entry */\n>> - struct string_list_item *item;\n>> -\n>> - item = string_list_insert_at_index(map, index, old_email);\n>> me = xcalloc(1, sizeof(struct mailmap_entry));\n>> me->namemap.strdup_strings = 1;\n>> me->namemap.cmp = namemap_cmp;\n>> --\n>> 2.2.0.rc3\n"},{"id":"252753","messageId":"xmqq7fyctcur.fsf@gitster.dls.corp.google.com","threadId":"38038","inReplyTo":"CAPig+cRNuuDDBV0-TwANuiv+f_c1mfXp2Q4rF1Sj5mtJoLoqHQ@mail.gmail.com","subject":"Re: [PATCHv3 2/3] mailmap: use higher level string list functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-01T00:47:40Z","receivedAt":"2014-12-01T00:47:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Thu, Nov 27, 2014 at 1:44 PM, Michael Blume wrote:\n>> The variable index seems to be unused/uninitialized now -- it's still\n>> printed in debug messages, but if I'm reading correctly, its contents are\n>> going to be nonsense.\n>\n> Nice catch.\n\nLet's do something like this squashed in, then.\n\n mailmap.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/mailmap.c b/mailmap.c\nindex 3b00a65..cb26af0 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -90,8 +90,8 @@ static void add_mapping(struct string_list *map,\n \t}\n \n \tif (old_name == NULL) {\n-\t\tdebug_mm(\"mailmap: adding (simple) entry for %s at index %d\\n\",\n-\t\t\t old_email, index);\n+\t\tdebug_mm(\"mailmap: adding (simple) entry for '%s'\\n\", old_email);\n+\n \t\t/* Replace current name and new email for simple entry */\n \t\tif (new_name) {\n \t\t\tfree(me->name);\n@@ -103,8 +103,7 @@ static void add_mapping(struct string_list *map,\n \t\t}\n \t} else {\n \t\tstruct mailmap_info *mi = xcalloc(1, sizeof(struct mailmap_info));\n-\t\tdebug_mm(\"mailmap: adding (complex) entry for %s at index %d\\n\",\n-\t\t\t old_email, index);\n+\t\tdebug_mm(\"mailmap: adding (complex) entry for '%s'\\n\", old_email);\n \t\tif (new_name)\n \t\t\tmi->name = xstrdup(new_name);\n \t\tif (new_email)\n"},{"id":"252757","messageId":"547BD36A.7050806@gmail.com","threadId":"38038","inReplyTo":"xmqq7fyctcur.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCHv3 2/3] mailmap: use higher level string list functions","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2014-12-01T02:33:14Z","receivedAt":"2014-12-01T02:33:14Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 30.11.2014 16:47, Junio C Hamano wrote:\n> Eric Sunshine writes:\n> \n>> On Thu, Nov 27, 2014 at 1:44 PM, Michael Blume wrote:\n>>> The variable index seems to be unused/uninitialized now -- it's still\n>>> printed in debug messages, but if I'm reading correctly, its contents are\n>>> going to be nonsense.\n>>\n>> Nice catch.\n> \n> Let's do something like this squashed in, then.\n\nMichael, thanks for catching that!\n\nJunio, the squash-in looks fine with me.\n\n> \n> mailmap.c | 7 +++----\n> 1 file changed, 3 insertions(+), 4 deletions(-)\n> \n> diff --git a/mailmap.c b/mailmap.c\n> index 3b00a65..cb26af0 100644\n> --- a/mailmap.c\n> +++ b/mailmap.c\n> @@ -90,8 +90,8 @@ static void add_mapping(struct string_list *map,\n> \t}\n> \n> \tif (old_name == NULL) {\n> -\t\tdebug_mm(\"mailmap: adding (simple) entry for %s at index %d\\n\",\n> -\t\t\t old_email, index);\n> +\t\tdebug_mm(\"mailmap: adding (simple) entry for '%s'\\n\", old_email);\n> +\n> \t\t/* Replace current name and new email for simple entry */\n> \t\tif (new_name) {\n> \t\t\tfree(me->name);\n> @@ -103,8 +103,7 @@ static void add_mapping(struct string_list *map,\n> \t\t}\n> \t} else {\n> \t\tstruct mailmap_info *mi = xcalloc(1, sizeof(struct mailmap_info));\n> -\t\tdebug_mm(\"mailmap: adding (complex) entry for %s at index %d\\n\",\n> -\t\t\t old_email, index);\n> +\t\tdebug_mm(\"mailmap: adding (complex) entry for '%s'\\n\", old_email);\n> \t\tif (new_name)\n> \t\t\tmi->name = xstrdup(new_name);\n> \t\tif (new_email)\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n"}]}