Re: Why our Valgrind reports suck

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Richard Guo <guofenglinux(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Why our Valgrind reports suck
Date: 2025-07-10 18:16:04
Message-ID: 1609690.1752171364@sss.pgh.pa.us
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Richard Guo <guofenglinux(at)gmail(dot)com> writes:
> I'm just skimming through the changes and happened to spot two minor
> things.

> In 0008:
> if (pq_mq_handle != NULL)
> + {
> shm_mq_detach(pq_mq_handle);
> + pfree(pq_mq_handle);
> + }
> pq_mq_handle = NULL;
> Maybe we could move "pq_mq_handle = NULL;" inside the if branch?
> Though I see we're doing it in your way on master.

Yeah, we could make it be like that. I was just trying to do the
minimal change from master.

> In 0015:
> I noticed that we're freeing the list returned from
> logicalrep_workers_find(). Should we do the same for the "workers"
> list in AtEOXact_LogicalRepWorkers()?

Seems probably unnecessary. AtEOXact functions should run in the
transaction's CurTransactionContext, which will be reset or deleted
once the transaction is done. If we were talking about a large amount
of memory it might be worth reclaiming sooner, but surely we are not.

> This is very useful work; I hope we can get it in soon.

Thanks for looking at it!

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2025-07-10 18:22:50 Re: Remaining dependency on setlocale()
Previous Message David E. Wheeler 2025-07-10 18:13:13 Re: PATCH: jsonpath string methods: lower, upper, initcap, l/r/btrim, replace, split_part