From: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: [PATCH] FIx resource leaks (pg_resetwal.c) |
Date: | 2020-04-23 18:40:21 |
Message-ID: | CAEudQAoJDNKNANhqTiavh9Q=FRS7DdKaFhdGjk+ZRUGuO_yCyw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Em qui., 23 de abr. de 2020 às 15:27, Andres Freund <andres(at)anarazel(dot)de>
escreveu:
> Hi,
>
> On 2020-04-23 15:20:59 -0300, Ranier Vilela wrote:
> > Per Coverity.
> >
> > read_controlfile alloc memory with pg_malloc and fail in releasing the
> > memory.
>
> Seriously, this is getting really ridiculous. You're posting badly
> vetted, often nearly verbatim, coverity reports. Many of them are
> obvious false positives. This is just producing noise.
>
I do not agree in any way. At the very least what I am reporting is
suspect. And if I already propose a solution even if it is not the best, it
is much better than being silent and missing the opportunity to fix a bug.
Ridiculous is your lack of education.
>
> Please stop.
>
I will ignore.
> > diff --git a/src/bin/pg_resetwal/pg_resetwal.c
> b/src/bin/pg_resetwal/pg_resetwal.c
> > index 233441837f..673ab0204c 100644
> > --- a/src/bin/pg_resetwal/pg_resetwal.c
> > +++ b/src/bin/pg_resetwal/pg_resetwal.c
> > @@ -608,6 +608,7 @@ read_controlfile(void)
> > len = read(fd, buffer, PG_CONTROL_FILE_SIZE);
> > if (len < 0)
> > {
> > + pg_free(buffer);
> > pg_log_error("could not read file \"%s\": %m",
> XLOG_CONTROL_FILE);
> > exit(1);
> > }
>
> There's an exit() two lines later, this is obviously not necessary.
>
Excess.
Did you read patch all over?
memcpy(&ControlFile, buffer, sizeof(ControlFile));
+ pg_free(buffer);
/* return false if WAL segment size is not valid */
if (!IsValidWalSegSize(ControlFile.xlog_seg_size))
@@ -644,6 +646,7 @@ read_controlfile(void)
return true;
}
+ pg_free(buffer);
/* Looks like it's a mess. */
pg_log_warning("pg_control exists but is broken or wrong version;
ignoring it");
Report for Coverity:
*** CID 1425435: Resource leaks (RESOURCE_LEAK)
/dll/postgres/src/bin/pg_resetwal/pg_resetwal.c: 650 in read_controlfile()
644
645 return true;
646 }
647
648 /* Looks like it's a mess. */
649 pg_log_warning("pg_control exists but is broken or wrong version;
ignoring it");
>>> CID 1425435: Resource leaks (RESOURCE_LEAK)
>>> Variable "buffer" going out of scope leaks the storage it points to.
650 return false;
651 }
652
653
654 /*
655 * Guess at pg_control values when we can't read
regards,
Ranier Vilela
Attachment | Content-Type | Size |
---|---|---|
fix_resource_leaks_pg_resetwal_v2.patch | application/octet-stream | 637 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2020-04-23 19:21:17 | Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators |
Previous Message | Andres Freund | 2020-04-23 18:27:03 | Re: [PATCH] FIx resource leaks (pg_resetwal.c) |