Re: [PATCH] commit-graph: fix start_delayed_progress() leak

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2025年6月4日 15:42,Patrick Steinhardt <ps@xxxxxx> 写道:
> 
> On Wed, Jun 04, 2025 at 03:11:15AM +0000, Lidong Yan via GitGitGadget wrote:
>> From: Lidong Yan <502024330056@xxxxxxxxxxxxxxxx>
>> 
>> In commit-graph.c:graph_write(), if read_one_commit() failed,
>> progress allocated in start_delayed_progress() will leak. Add
>> stop_progress() before goto cleanup.
>> 
>> Signed-off-by: Lidong Yan <502024330056@xxxxxxxxxxxxxxxx>
> 
> Nit: it might make sense to send multiple patches that are related, like
> your memory leak fixes, in the same patch series. That makes it a bit
> easier for reviewers to group together related reviews.

Got it, though I think this is the last time I send leak-fix patches.

> 
>> diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
>> index a783a86e797..ee48980248f 100644
>> --- a/builtin/commit-graph.c
>> +++ b/builtin/commit-graph.c
>> @@ -311,6 +311,7 @@ static int graph_write(int argc, const char **argv, const char *prefix,
>> while (strbuf_getline(&buf, stdin) != EOF) {
>> if (read_one_commit(&commits, progress, buf.buf)) {
>> result = 1;
>> + stop_progress(&progress);
> 
> This function calls `stop_progress_msg()`, which knows to exit in case
> `*progress` is a NULL pointer. We thus don't have to guard this line
> with `if (progress)`. So the patch looks good to me, thanks!
> 
> Patrick
> 

Thanks,
Lidong






[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux