threads / patch / 38038

v2, 3 partsmailmap: use higher level string list functions

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

## tl;dr

6 messages between Nov 24, 2014 and Dec 1, 2014. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Stefan Beller· Nov 24, 2014, 21:52 UTC · lore

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(-)
Show changes to mailmap.c +4 −8
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· Nov 25, 2014, 02:36 UTC · re: Stefan Beller · lore

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

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/
Show 8 quoted lines
> 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· Nov 25, 2014, 03:44 UTC · re: Eric Sunshine · lore

[PATCHv3 2/3] mailmap: use higher level string list functions

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(-)
Show changes to mailmap.c +4 −8
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· Nov 27, 2014, 19:57 UTC · lore

Re: [PATCHv3 2/3] mailmap: use higher level string list functions

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
Show 55 quoted lines
> 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· Dec 1, 2014, 00:47 UTC · re: Eric Sunshine · lore

Re: [PATCHv3 2/3] mailmap: use higher level string list functions

Eric Sunshine <sunshine@sunshineco.com> writes:
Show 6 quoted lines
> 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(-)
Show changes to mailmap.c +3 −4
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· Dec 1, 2014, 02:33 UTC · re: Junio C Hamano · lore

Re: [PATCHv3 2/3] mailmap: use higher level string list functions

On 30.11.2014 16:47, Junio C Hamano wrote:
Show 10 quoted lines
> 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.
Show 34 quoted lines
> 
>  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
>

← back to recent threads