git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] merge-recursive: use fspathcmp() in path_hashmap_cmp()

From
René Scharfe <l.s.r@web.de>
Date
Aug 30, 2021, 15:09 UTC
Message-ID
<60e9baf4-4e76-71a8-e6bb-1af87486994d@web.de>
In-Reply-To
<YSvsQcGNpCMZwS8o@nand.local>
Am 29.08.21 um 22:21 schrieb Taylor Blau:
Show 47 quoted lines
> Hi René,
>
> On Sat, Aug 28, 2021 at 11:30:49PM +0200, René Scharfe wrote:
>> Call fspathcmp() instead of open-coding it.  This shortens the code and
>> makes it less repetitive.
>>
>> Signed-off-by: René Scharfe <l.s.r@web.de>
>> ---
>>  merge-recursive.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/merge-recursive.c b/merge-recursive.c
>> index 3355d50e8a..840599fd53 100644
>> --- a/merge-recursive.c
>> +++ b/merge-recursive.c
>> @@ -55,10 +55,7 @@ static int path_hashmap_cmp(const void *cmp_data,
>>  	a = container_of(eptr, const struct path_hashmap_entry, e);
>>  	b = container_of(entry_or_key, const struct path_hashmap_entry, e);
>>
>> -	if (ignore_case)
>> -		return strcasecmp(a->path, key ? key : b->path);
>> -	else
>> -		return strcmp(a->path, key ? key : b->path);
>> +	return fspathcmp(a->path, key ? key : b->path);
>>  }
>
> Looks obviously right to me. I found another spot in
> t/helper/test-hashmap.c:test_entry_cmp() that could be cleaned up in the
> same way. But this looks fine with or without the following diff:
>
> diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c
> index 36ff07bd4b..ab34bdfecd 100644
> --- a/t/helper/test-hashmap.c
> +++ b/t/helper/test-hashmap.c
> @@ -28,10 +28,7 @@ static int test_entry_cmp(const void *cmp_data,
>         e1 = container_of(eptr, const struct test_entry, ent);
>         e2 = container_of(entry_or_key, const struct test_entry, ent);
>
> -       if (ignore_case)
> -               return strcasecmp(e1->key, key ? key : e2->key);
> -       else
> -               return strcmp(e1->key, key ? key : e2->key);
> +       return fspathcmp(e1->key, key ? key : e2->key);
>  }
>
>  static struct test_entry *alloc_test_entry(unsigned int hash,
>

That's a local variable named "ignore_case", not the one declared in environment.c that fspathcmp() uses, so this would change the behavior. The helper code does not include cache.h, so this is not even a case of variable shadowing, just two different variables for similar purposes in different places having the same name.

René
Previous: Junio C HamanoNext: Jeff King
Message 5 of 15 in “merge-recursive: use fspathcmp() in path_hashmap_cmp()”
  1. merge-recursive: use fspathcmp() in path_hashmap_cmp()René Scharfe, Aug 28, 2021
  2. Taylor BlauAug 29, 2021
  3. Jeff KingAug 29, 2021
  4. Junio C HamanoAug 30, 2021
  5. René ScharfeAug 30, 2021
  6. Jeff KingAug 30, 2021
  7. Junio C HamanoAug 30, 2021
  8. René ScharfeAug 30, 2021
  9. Jeff KingAug 30, 2021
  10. René ScharfeSep 11, 2021
  11. Johannes SchindelinSep 13, 2021
  12. Jeff KingSep 13, 2021
  13. Junio C HamanoSep 13, 2021
  14. Johannes SchindelinSep 14, 2021
  15. Jeff KingSep 14, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.