Re: [PATCH 2/2] diff-no-index: release prefixed filenames
- From
René Scharfe <l.s.r@web.de>
- Date
- Sep 7, 2022, 11:19 UTC
- Message-ID
- <90d7af20-0563-3e09-f7dc-f69f8d473928@web.de>
- In-Reply-To
- <n575rrn9-5p4o-r40o-snsq-197p242q10p5@tzk.qr>
Am 07.09.22 um 12:03 schrieb Johannes Schindelin:
Show 36 quoted lines
> Hi René,
>
> On Tue, 6 Sep 2022, René Scharfe wrote:
>
>> diff --git a/diff-no-index.c b/diff-no-index.c
>> index a3683d8a04..35809f26d7 100644
>> --- a/diff-no-index.c
>> +++ b/diff-no-index.c
>> @@ -245,6 +245,7 @@ int diff_no_index(struct rev_info *revs,
>> int i, no_index;
>> int ret = 1;
>> const char *paths[2];
>> + char *to_free[2] = { 0 };
>> struct strbuf replacement = STRBUF_INIT;
>> const char *prefix = revs->prefix;
>> struct option no_index_options[] = {
>> @@ -274,7 +275,7 @@ int diff_no_index(struct rev_info *revs,
>> */
>> p = file_from_standard_input;
>> else if (prefix)
>> - p = prefix_filename(prefix, p);
>> + p = to_free[i] = prefix_filename(prefix, p);
>> paths[i] = p;
>> }
>>
>> @@ -308,6 +309,8 @@ int diff_no_index(struct rev_info *revs,
>> ret = diff_result_code(&revs->diffopt, 0);
>>
>> out:
>> + for (i = 0; i < 2; i++)
>> + free(to_free[i]);
>
> Heh. That's long-hand for
>
> free(to_free[0]);
> free(to_free[1]);Had that before, but it's repetitive and more importantly this loop matches the first one.
> If you do want to have that loop, please replace the hard-coded 2 by > `ARRAY_SIZE(to_free)`.
The two is hard-coded in other places explicitly as well and implied in fixup_paths(). The root cause is not any array size but the design decision to require exactly two things to compare. A reader would need to know that. We could sure use ARRAY_SIZE(paths) in the declaration of to_free and ARRAY_SIZE(to_free) in the loop to at least not add more instances of that magic number and make the code understandable without seeing the bigger picture.
Show 11 quoted lines
> Otherwise, both patches look fine to me. > > Thanks! > Dscho > >> strbuf_release(&replacement); >> return ret; >> } >> -- >> 2.37.2 >>