From: | Aleksander Alekseev <aleksander(at)tigerdata(dot)com> |
---|---|
To: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | Re: [PATCH] Silence Valgrind about SelectConfigFiles() |
Date: | 2025-08-14 11:09:35 |
Message-ID: | CAJ7c6TMFgp5p9Erdh5PODXbwc2MT8ogiZQx3mcN0O3TXuOpoAQ@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Tom,
> Yeah, I noticed that too, and it does offend my inner neatnik.
>
> Instead of what you did, I'd be inclined to add
>
> free(configdir);
>
> return true;
> +
> +fail:
> + free(configdir);
> +
> + return false;
> }
>
> and then s/return false/goto fail/ throughout, so as to avoid
> duplicating the free() calls. It's a minor point as things stand,
> but if more cleanup gets added to the function I think it'd be
> easier to maintain this way.
Makes sense. Here is the corrected patch v2.
> Huh ... don't quite see where in that recipe we'd reach a
> SelectConfigFiles error exit.
How exactly we reach this code patch is a good question. I tried to
understand the exact conditions by using my steps to reproduce and an
ancient debugging technique with sleep(), elog() and `watch` - see
trick.txt. Unfortunately I was not able to reproduce it again nor
under Valgrind nor without it. I guess it means that either I did
something differently before or the right conditions are met under
rare circumstances.
Attachment | Content-Type | Size |
---|---|---|
trick.txt | text/plain | 1.5 KB |
v2-0001-Fix-memory-leaks-in-SelectConfigFiles.patch | text/x-patch | 2.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Shinya Kato | 2025-08-14 11:12:55 | Add mode column to pg_stat_progress_vacuum |
Previous Message | Alastair Turner | 2025-08-14 10:56:26 | Re: Proposal: Conflict log history table for Logical Replication |