Skip to content

Commit 0a798ea

Browse files
committed
gvfs-helper-client: be careful about process entries
The gh_client__find_long_running_process() method returns a gh_server__process pointer when finding a process. However, that could be contained within the hashtable of long-running processes _or_ a brand-new struct due to creating a new process. This means that we are leaking data in these cases where a struct is created directly. Add a new signal to say whether or not this pointer should be removed. Note that there are some error cases in the callers that already FREE_AND_NULL() the value, but that does not seem to be a problem in our test scripts. This change is necessary to help t5799-gvfs-helper.sh pass when compiled with SANITIZE=leak. Signed-off-by: Derrick Stolee <[email protected]>
1 parent 82b0a7e commit 0a798ea

File tree

1 file changed

+18
-4
lines changed

1 file changed

+18
-4
lines changed

gvfs-helper-client.c

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,8 @@ static void gh_client__choose_odb(void)
371371
}
372372

373373
static struct gh_server__process *gh_client__find_long_running_process(
374-
unsigned int cap_needed)
374+
unsigned int cap_needed,
375+
int *must_free)
375376
{
376377
struct gh_server__process *entry;
377378
struct strvec argv = STRVEC_INIT;
@@ -416,6 +417,7 @@ static struct gh_server__process *gh_client__find_long_running_process(
416417
if (!entry) {
417418
entry = xmalloc(sizeof(*entry));
418419
entry->supported_capabilities = 0;
420+
*must_free = 1;
419421

420422
if (subprocess_start_strvec(&gh_server__subprocess_map,
421423
&entry->subprocess, 1,
@@ -475,13 +477,14 @@ int gh_client__drain_queue(enum gh_client__created *p_ghc)
475477
int nr_loose = 0;
476478
int nr_packfile = 0;
477479
int err = 0;
480+
int must_free = 0;
478481

479482
*p_ghc = GHC__CREATED__NOTHING;
480483

481484
if (!gh_client__oidset_count)
482485
return 0;
483486

484-
entry = gh_client__find_long_running_process(CAP_OBJECTS);
487+
entry = gh_client__find_long_running_process(CAP_OBJECTS, &must_free);
485488
if (!entry)
486489
return -1;
487490

@@ -511,6 +514,9 @@ int gh_client__drain_queue(enum gh_client__created *p_ghc)
511514
oidset_clear(&gh_client__oidset_queued);
512515
gh_client__oidset_count = 0;
513516

517+
if (must_free)
518+
FREE_AND_NULL(entry);
519+
514520
return err;
515521
}
516522

@@ -526,6 +532,7 @@ int gh_client__get_immediate(const struct object_id *oid,
526532
int nr_loose = 0;
527533
int nr_packfile = 0;
528534
int err = 0;
535+
int must_free = 0;
529536

530537
/*
531538
* Keep this trace as a printf only, so that it goes to the
@@ -536,7 +543,7 @@ int gh_client__get_immediate(const struct object_id *oid,
536543
if (trace2_is_enabled())
537544
trace2_printf("gh_client__get_immediate: %s", oid_to_hex(oid));
538545

539-
entry = gh_client__find_long_running_process(CAP_OBJECTS);
546+
entry = gh_client__find_long_running_process(CAP_OBJECTS, &must_free);
540547
if (!entry)
541548
return -1;
542549

@@ -561,6 +568,9 @@ int gh_client__get_immediate(const struct object_id *oid,
561568

562569
trace2_region_leave("gh-client", "objects/get", the_repository);
563570

571+
if (must_free)
572+
FREE_AND_NULL(entry);
573+
564574
return err;
565575
}
566576

@@ -580,8 +590,9 @@ int gh_client__prefetch(timestamp_t seconds_since_epoch,
580590
int nr_loose = 0;
581591
int nr_packfile = 0;
582592
int err = 0;
593+
int must_free = 0;
583594

584-
entry = gh_client__find_long_running_process(CAP_OBJECTS);
595+
entry = gh_client__find_long_running_process(CAP_OBJECTS, &must_free);
585596
if (!entry)
586597
return -1;
587598

@@ -613,5 +624,8 @@ int gh_client__prefetch(timestamp_t seconds_since_epoch,
613624
if (nr_packfiles_received)
614625
*nr_packfiles_received = nr_packfile;
615626

627+
if (must_free)
628+
FREE_AND_NULL(entry);
629+
616630
return err;
617631
}

0 commit comments

Comments
 (0)