On Thu, Jul 21, 2022 at 02:43:03PM +0800, Japin Li wrote:
> I attached a patch for v14 [1] based on master, if you want to apply it,
> please consider reviewing it.
We are talking about a few hundred bytes leaked each time, so this
does not worry me much in the older branches, honestly.
--
Michael
On Thu, 21 Jul 2022 at 14:23, Alvaro Herrera wrote:
> On 2022-Jul-21, Japin Li wrote:
>
>> On Thu, 21 Jul 2022 at 09:48, Michael Paquier wrote:
>
>> > We are talking about 256 bytes being leaked in each loop when a
>> > validation pattern or when a query fails, so I don't see a strong
>> > argu
On 2022-Jul-21, Japin Li wrote:
> On Thu, 21 Jul 2022 at 09:48, Michael Paquier wrote:
> > We are talking about 256 bytes being leaked in each loop when a
> > validation pattern or when a query fails, so I don't see a strong
> > argument in manipulating 10~14 more than necessary for this amount
On Thu, 21 Jul 2022 at 09:48, Michael Paquier wrote:
> On Thu, Jul 21, 2022 at 09:10:43AM +0800, Japin Li wrote:
>> Yeah, we should take care of the backpatch risk. However, I think
>> it makes sense to backpatch.
>
> We are talking about 256 bytes being leaked in each loop when a
> validation
On Thu, Jul 21, 2022 at 09:10:43AM +0800, Japin Li wrote:
> Yeah, we should take care of the backpatch risk. However, I think
> it makes sense to backpatch.
We are talking about 256 bytes being leaked in each loop when a
validation pattern or when a query fails, so I don't see a strong
argument i
On Wed, 20 Jul 2022 at 21:54, Tom Lane wrote:
> Japin Li writes:
>> Thanks for updating the patch. It looks good. However, it cannot be
>> applied on 14 stable. The attached patches are for 10-14.
>
> While I think this is good cleanup, I'm doubtful that any of these
> leaks are probable eno
On Wed, Jul 20, 2022 at 10:05:47AM +0200, Alvaro Herrera wrote:
> More on the same tune.
Thanks. I have noticed that as well. I'll include all that in the
set.
--
Michael
signature.asc
Description: PGP signature
Japin Li writes:
> Thanks for updating the patch. It looks good. However, it cannot be
> applied on 14 stable. The attached patches are for 10-14.
While I think this is good cleanup, I'm doubtful that any of these
leaks are probable enough to be worth back-patching into stable
branches. The r
On Wed, 20 Jul 2022 at 14:21, tanghy.f...@fujitsu.com
wrote:
> On Wednesday, July 20, 2022 12:52 PM, Michael Paquier
> wrote:
>> What about the argument of upthread where we could use a goto in
>> functions where there are multiple pattern validation checks? Per se
>> v4 attached.
>
> Thanks
More on the same tune.
--
Álvaro HerreraBreisgau, Deutschland — https://www.EnterpriseDB.com/
"This is what I like so much about PostgreSQL. Most of the surprises
are of the "oh wow! That's cool" Not the "oh shit!" kind. :)"
Scott Marlowe, http://archives.postgresql.org/pgsql-admin/2
Thanks for your explanation, this time I know how it works, thanks ;)
On Wed, Jul 20, 2022 at 3:04 PM tanghy.f...@fujitsu.com
wrote:
>
> > On Wed, Jul 20, 2022 at 12:51:24PM +0800, Junwang Zhao wrote:
> > > Though the patch looks good, I myself think the patch should be edited
> > > and submitted
> On Wed, Jul 20, 2022 at 12:51:24PM +0800, Junwang Zhao wrote:
> > Though the patch looks good, I myself think the patch should be edited
> > and submitted by Tang
> > It's easy to attach a fixed patch based on the comments of the thread,
> > but coins should be
> > given to Tang since he is the f
On Wednesday, July 20, 2022 12:52 PM, Michael Paquier
wrote:
> What about the argument of upthread where we could use a goto in
> functions where there are multiple pattern validation checks? Per se
> v4 attached.
Thanks for your kindly remind and modification.
I checked v4 patch, it looks good
Got it, thanks!
On Wed, Jul 20, 2022 at 1:14 PM Michael Paquier wrote:
>
> On Wed, Jul 20, 2022 at 12:51:24PM +0800, Junwang Zhao wrote:
> > Though the patch looks good, I myself think the patch should be edited
> > and submitted by Tang
> > It's easy to attach a fixed patch based on the comments
On Wed, Jul 20, 2022 at 12:51:24PM +0800, Junwang Zhao wrote:
> Though the patch looks good, I myself think the patch should be edited
> and submitted by Tang
> It's easy to attach a fixed patch based on the comments of the thread,
> but coins should be
> given to Tang since he is the first one to
-1
Though the patch looks good, I myself think the patch should be edited
and submitted by Tang
It's easy to attach a fixed patch based on the comments of the thread,
but coins should be
given to Tang since he is the first one to find the mem leak.
No offense, but that's what I think how open sou
On Wed, 20 Jul 2022 at 11:51, Michael Paquier wrote:
> On Wed, Jul 20, 2022 at 03:14:35AM +, tanghy.f...@fujitsu.com wrote:
>> Your fix LGTM so please allow me to merge it in the attached patch.
>> Based on your rebased version, now this new patch version is V3.
>
> What about the argument
On Wed, Jul 20, 2022 at 03:14:35AM +, tanghy.f...@fujitsu.com wrote:
> Your fix LGTM so please allow me to merge it in the attached patch.
> Based on your rebased version, now this new patch version is V3.
What about the argument of upthread where we could use a goto in
functions where there
On Tuesday, July 19, 2022 7:41 PM, Japin Li wrote:
> After looking around, I found psql/describe.c also has some memory
> leaks,
> attached a patch to fix these leaks.
Thanks for your check and improvement.
Your fix LGTM so please allow me to merge it in the attached patch.
Based on your rebased
On Tue, Jul 19, 2022 at 09:43:21AM -0700, Mark Dilger wrote:
> This looks ok, but comments down-thread seem reasonable, so I
> suspect a new patch will be needed. Would you like to author it, or
> would you prefer that I, as the guilty party, do so?
If any of you could update the patch, that wou
On Tue, Jul 19, 2022 at 12:36:31PM -0400, Tom Lane wrote:
> Yeah, it's old school, but please let's not have a few functions that
> do it randomly differently from all their neighbors.
True enough. And it is not like we should free the PQExpBuffer given
by the caller in validateSQLNamePattern().
> On Jul 19, 2022, at 2:02 AM, tanghy.f...@fujitsu.com wrote:
>
> I think there is a newly introduced memory leak in your patch d2d3547.
I agree. Thanks for noticing, and for the patch!
> Try to fix it in the attached patch.
> Kindly to have a check.
This looks ok, but comments down-thread
Andres Freund writes:
> On 2022-07-19 21:08:53 +0800, Japin Li wrote:
>> +{
>> +termPQExpBuffer(&buf);
>> return false;
>> +}
> Adding copy over copy of this same block doesn't seem great. Can we instead
> add a helper for it or such?
The usual style in these fil
Hi,
On 2022-07-19 21:08:53 +0800, Japin Li wrote:
> From b2bcc3a1bac67b8b414f2025607f8dd35e096289 Mon Sep 17 00:00:00 2001
> From: Japin Li
> Date: Tue, 19 Jul 2022 18:27:25 +0800
> Subject: [PATCH v2 1/1] Fix the memory leak in psql describe
>
> ---
> src/bin/psql/describe.c | 168
On Tue, 19 Jul 2022 at 20:32, Michael Paquier wrote:
> On Tue, Jul 19, 2022 at 06:41:13PM +0800, Japin Li wrote:
>> After looking around, I found psql/describe.c also has some memory leaks,
>> attached a patch to fix these leaks.
>
> Indeed. There are quite a bit of them, so let's fix all that.
On Tue, Jul 19, 2022 at 06:41:13PM +0800, Japin Li wrote:
> After looking around, I found psql/describe.c also has some memory leaks,
> attached a patch to fix these leaks.
Indeed. There are quite a bit of them, so let's fix all that. You
have missed a couple of code paths in objectDescription()
On Tue, 19 Jul 2022 at 17:02, tanghy.f...@fujitsu.com
wrote:
> Hi
>
> I think there is a newly introduced memory leak in your patch d2d3547.
> Try to fix it in the attached patch.
> Kindly to have a check.
>
Yeah, it leaks, and the patch can fix it.
After looking around, I found psql/describe
Hi
I think there is a newly introduced memory leak in your patch d2d3547.
Try to fix it in the attached patch.
Kindly to have a check.
Regards,
Tang
v1-0001-fix-the-memory-leak-in-validateSQLNamePattern.patch
Description: v1-0001-fix-the-memory-leak-in-validateSQLNamePattern.patch
28 matches
Mail list logo
