# [PATCH 2/3 v2] mailmap: use higher level string list functions

6 messages from 2014-11-24 to 2014-12-01. Participants: Stefan Beller, Eric Sunshine, Junio C Hamano.
Thread: https://gitlist.dev/t/38038

## Stefan Beller, 2014-11-24 21:52

Subject: [PATCH 2/3 v2] mailmap: use higher level string list functions
Message-ID: <1416865932-18285-1-git-send-email-sbeller@google.com>
URL: https://gitlist.dev/e/1416865932-18285-1-git-send-email-sbeller%40google.com
In-Reply-To: <xmqq3898w9v4.fsf@gitster.dls.corp.google.com>

```
No functional changes intended. This commit makes user of higher level
and better documented functions of the string list API, so the code is
more understandable.

Note that also the required computational amount should not change
in principal as we need to look up the item no matter if it is already
part of the list or not. Once looked up, insertion comes for free.

Signed-off-by: Stefan Beller <sbeller@google.com>
---

Changes since Version 1:
* Remove declaration-after-statement.

 mailmap.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/mailmap.c b/mailmap.c
index 81890a6..3b00a65 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -71,6 +71,7 @@ static void add_mapping(struct string_list *map,
 			char *old_name, char *old_email)
 {
 	struct mailmap_entry *me;
+	struct string_list_item *item;
 	int index;
 
 	if (old_email == NULL) {
@@ -78,15 +79,10 @@ static void add_mapping(struct string_list *map,
 		new_email = NULL;
 	}
 
-	if ((index = string_list_find_insert_index(map, old_email, 1)) < 0) {
-		/* mailmap entry exists, invert index value */
-		index = -1 - index;
-		me = (struct mailmap_entry *)map->items[index].util;
+	item = string_list_insert(map, old_email);
+	if (item->util) {
+		me = (struct mailmap_entry *)item->util;
 	} else {
-		/* create mailmap entry */
-		struct string_list_item *item;
-
-		item = string_list_insert_at_index(map, index, old_email);
 		me = xcalloc(1, sizeof(struct mailmap_entry));
 		me->namemap.strdup_strings = 1;
 		me->namemap.cmp = namemap_cmp;
-- 
2.2.0.rc3

```

## Eric Sunshine, 2014-11-25 02:36

Subject: Re: [PATCH 2/3 v2] mailmap: use higher level string list functions
Message-ID: <CAPig+cQ_4A-0LOgXXG5qLeHOev+g8KMq5osKz34AFijGuyRidQ@mail.gmail.com>
URL: https://gitlist.dev/e/CAPig%2BcQ_4A-0LOgXXG5qLeHOev%2Bg8KMq5osKz34AFijGuyRidQ%40mail.gmail.com
In-Reply-To: <1416865932-18285-1-git-send-email-sbeller@google.com>

```
On Mon, Nov 24, 2014 at 4:52 PM, Stefan Beller <sbeller@google.com> wrote:
> No functional changes intended. This commit makes user of higher level

s/user/use/

> and better documented functions of the string list API, so the code is
> more understandable.
>
> Note that also the required computational amount should not change
> in principal as we need to look up the item no matter if it is already
> part of the list or not. Once looked up, insertion comes for free.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>

```

## Stefan Beller, 2014-11-25 03:44

Subject: [PATCHv3 2/3] mailmap: use higher level string list functions
Message-ID: <1416887054-27204-1-git-send-email-sbeller@google.com>
URL: https://gitlist.dev/e/1416887054-27204-1-git-send-email-sbeller%40google.com
In-Reply-To: <CAPig+cQ_4A-0LOgXXG5qLeHOev+g8KMq5osKz34AFijGuyRidQ@mail.gmail.com>

```
No functional changes intended. This commit makes use of higher level
and better documented functions of the string list API, so the code is
more understandable.

Note that also the required computational amount should not change
in principal as we need to look up the item no matter if it is already
part of the list or not. Once looked up, insertion comes for free.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
Changes since Version 1:
* Remove declaration-after-statement.

Changes Version 1 to Version 2:
* typo in commit message

 mailmap.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/mailmap.c b/mailmap.c
index 81890a6..3b00a65 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -71,6 +71,7 @@ static void add_mapping(struct string_list *map,
 			char *old_name, char *old_email)
 {
 	struct mailmap_entry *me;
+	struct string_list_item *item;
 	int index;
 
 	if (old_email == NULL) {
@@ -78,15 +79,10 @@ static void add_mapping(struct string_list *map,
 		new_email = NULL;
 	}
 
-	if ((index = string_list_find_insert_index(map, old_email, 1)) < 0) {
-		/* mailmap entry exists, invert index value */
-		index = -1 - index;
-		me = (struct mailmap_entry *)map->items[index].util;
+	item = string_list_insert(map, old_email);
+	if (item->util) {
+		me = (struct mailmap_entry *)item->util;
 	} else {
-		/* create mailmap entry */
-		struct string_list_item *item;
-
-		item = string_list_insert_at_index(map, index, old_email);
 		me = xcalloc(1, sizeof(struct mailmap_entry));
 		me->namemap.strdup_strings = 1;
 		me->namemap.cmp = namemap_cmp;
-- 
2.2.0.rc3

```

## Eric Sunshine, 2014-11-27 19:57

Subject: Re: [PATCHv3 2/3] mailmap: use higher level string list functions
Message-ID: <CAPig+cRNuuDDBV0-TwANuiv+f_c1mfXp2Q4rF1Sj5mtJoLoqHQ@mail.gmail.com>
URL: https://gitlist.dev/e/CAPig%2BcRNuuDDBV0-TwANuiv%2Bf_c1mfXp2Q4rF1Sj5mtJoLoqHQ%40mail.gmail.com
In-Reply-To: <CAO2U3QjNua2HvJKLnq80mPFEp931yLzHKENKo-LHm4CFZWRhBA@mail.gmail.com>

```
On Thu, Nov 27, 2014 at 1:44 PM, Michael Blume <blume.mike@gmail.com> wrote:
> The variable index seems to be unused/uninitialized now -- it's still
> printed in debug messages, but if I'm reading correctly, its contents are
> going to be nonsense.

Nice catch.

(Aside: Please don't top-post on this list [1].)

[1]: https://lkml.org/lkml/2005/1/11/111


> On Mon Nov 24 2014 at 7:48:27 PM Stefan Beller <sbeller@google.com> wrote:
>>
>> No functional changes intended. This commit makes use of higher level
>> and better documented functions of the string list API, so the code is
>> more understandable.
>>
>> Note that also the required computational amount should not change
>> in principal as we need to look up the item no matter if it is already
>> part of the list or not. Once looked up, insertion comes for free.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>> Changes since Version 1:
>> * Remove declaration-after-statement.
>>
>> Changes Version 1 to Version 2:
>> * typo in commit message
>>
>>  mailmap.c | 12 ++++--------
>>  1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/mailmap.c b/mailmap.c
>> index 81890a6..3b00a65 100644
>> --- a/mailmap.c
>> +++ b/mailmap.c
>> @@ -71,6 +71,7 @@ static void add_mapping(struct string_list *map,
>>                         char *old_name, char *old_email)
>>  {
>>         struct mailmap_entry *me;
>> +       struct string_list_item *item;
>>         int index;
>>
>>         if (old_email == NULL) {
>> @@ -78,15 +79,10 @@ static void add_mapping(struct string_list *map,
>>                 new_email = NULL;
>>         }
>>
>> -       if ((index = string_list_find_insert_index(map, old_email, 1)) <
>> 0) {
>> -               /* mailmap entry exists, invert index value */
>> -               index = -1 - index;
>> -               me = (struct mailmap_entry *)map->items[index].util;
>> +       item = string_list_insert(map, old_email);
>> +       if (item->util) {
>> +               me = (struct mailmap_entry *)item->util;
>>         } else {
>> -               /* create mailmap entry */
>> -               struct string_list_item *item;
>> -
>> -               item = string_list_insert_at_index(map, index, old_email);
>>                 me = xcalloc(1, sizeof(struct mailmap_entry));
>>                 me->namemap.strdup_strings = 1;
>>                 me->namemap.cmp = namemap_cmp;
>> --
>> 2.2.0.rc3

```

## Junio C Hamano, 2014-12-01 00:47

Subject: Re: [PATCHv3 2/3] mailmap: use higher level string list functions
Message-ID: <xmqq7fyctcur.fsf@gitster.dls.corp.google.com>
URL: https://gitlist.dev/e/xmqq7fyctcur.fsf%40gitster.dls.corp.google.com
In-Reply-To: <CAPig+cRNuuDDBV0-TwANuiv+f_c1mfXp2Q4rF1Sj5mtJoLoqHQ@mail.gmail.com>

```
Eric Sunshine <sunshine@sunshineco.com> writes:

> On Thu, Nov 27, 2014 at 1:44 PM, Michael Blume <blume.mike@gmail.com> wrote:
>> The variable index seems to be unused/uninitialized now -- it's still
>> printed in debug messages, but if I'm reading correctly, its contents are
>> going to be nonsense.
>
> Nice catch.

Let's do something like this squashed in, then.

 mailmap.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/mailmap.c b/mailmap.c
index 3b00a65..cb26af0 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -90,8 +90,8 @@ static void add_mapping(struct string_list *map,
 	}
 
 	if (old_name == NULL) {
-		debug_mm("mailmap: adding (simple) entry for %s at index %d\n",
-			 old_email, index);
+		debug_mm("mailmap: adding (simple) entry for '%s'\n", old_email);
+
 		/* Replace current name and new email for simple entry */
 		if (new_name) {
 			free(me->name);
@@ -103,8 +103,7 @@ static void add_mapping(struct string_list *map,
 		}
 	} else {
 		struct mailmap_info *mi = xcalloc(1, sizeof(struct mailmap_info));
-		debug_mm("mailmap: adding (complex) entry for %s at index %d\n",
-			 old_email, index);
+		debug_mm("mailmap: adding (complex) entry for '%s'\n", old_email);
 		if (new_name)
 			mi->name = xstrdup(new_name);
 		if (new_email)

```

## Stefan Beller, 2014-12-01 02:33

Subject: Re: [PATCHv3 2/3] mailmap: use higher level string list functions
Message-ID: <547BD36A.7050806@gmail.com>
URL: https://gitlist.dev/e/547BD36A.7050806%40gmail.com
In-Reply-To: <xmqq7fyctcur.fsf@gitster.dls.corp.google.com>

```
On 30.11.2014 16:47, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
>> On Thu, Nov 27, 2014 at 1:44 PM, Michael Blume <blume.mike@gmail.com> wrote:
>>> The variable index seems to be unused/uninitialized now -- it's still
>>> printed in debug messages, but if I'm reading correctly, its contents are
>>> going to be nonsense.
>>
>> Nice catch.
> 
> Let's do something like this squashed in, then.

Michael, thanks for catching that!

Junio, the squash-in looks fine with me.

> 
>  mailmap.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/mailmap.c b/mailmap.c
> index 3b00a65..cb26af0 100644
> --- a/mailmap.c
> +++ b/mailmap.c
> @@ -90,8 +90,8 @@ static void add_mapping(struct string_list *map,
>  	}
>  
>  	if (old_name == NULL) {
> -		debug_mm("mailmap: adding (simple) entry for %s at index %d\n",
> -			 old_email, index);
> +		debug_mm("mailmap: adding (simple) entry for '%s'\n", old_email);
> +
>  		/* Replace current name and new email for simple entry */
>  		if (new_name) {
>  			free(me->name);
> @@ -103,8 +103,7 @@ static void add_mapping(struct string_list *map,
>  		}
>  	} else {
>  		struct mailmap_info *mi = xcalloc(1, sizeof(struct mailmap_info));
> -		debug_mm("mailmap: adding (complex) entry for %s at index %d\n",
> -			 old_email, index);
> +		debug_mm("mailmap: adding (complex) entry for '%s'\n", old_email);
>  		if (new_name)
>  			mi->name = xstrdup(new_name);
>  		if (new_email)
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

```
