Simplifies checks (src/bin/pg_dump/pg_backup_archiver.c)
Hi.
Per Coverity.
Coverity has some alerts about the function *RestoreArchive*,
arguing that there are null variable dereferences.
I haven't done in-depth research to determine if this is correct or not.
However, the code can be simplified to a single check that would silence
notifications and protect against this issue.
patch attached.
best regards,
Ranier Vilela
Attachments:
simplifies-checks-pg_basebackup_archiver.patchapplication/octet-stream; name=simplifies-checks-pg_basebackup_archiver.patchDownload+6-2
On 9 Mar 2026, at 14:07, Ranier Vilela <ranier.vf@gmail.com> wrote:
Coverity has some alerts about the function *RestoreArchive*,
arguing that there are null variable dereferences.
Coverity is very good at generating false positives.
I haven't done in-depth research to determine if this is correct or not.
However, the code can be simplified to a single check that would silence notifications and protect against this issue.
You probably should do that research. What if te is actually null and with
your patch the code accepts it rather than crashes. Does that affect the
integrity of the dump? Will it crash soon after? Is te allowed to be null?
--
Daniel Gustafsson
On Mon, 09 Mar 2026 at 10:07, Ranier Vilela <ranier.vf@gmail.com> wrote:
Hi.
Per Coverity.
Coverity has some alerts about the function *RestoreArchive*,
arguing that there are null variable dereferences.I haven't done in-depth research to determine if this is correct or not.
However, the code can be simplified to a single check that would silence notifications and protect against this issue.patch attached.
According to _allocAH(), te cannot be null.
AH->toc = pg_malloc0_object(TocEntry);
AH->toc->next = AH->toc;
AH->toc->prev = AH->toc;
So we can safely remove its null check.
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index df8a69d3b79..137e5850f8e 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -767,7 +767,7 @@ RestoreArchive(Archive *AHX, bool append_data)
continue; /* ignore if not to be dumped at all */
/* Skip if no-tablespace is given. */
- if (ropt->noTablespace && te && te->desc &&
+ if (ropt->noTablespace && te->desc &&
(strcmp(te->desc, "TABLESPACE") == 0))
continue;
@@ -775,7 +775,7 @@ RestoreArchive(Archive *AHX, bool append_data)
* Skip DROP DATABASE/ROLES/TABLESPACE if we didn't specify
* --clean
*/
- if (!ropt->dropSchema && te && te->desc &&
+ if (!ropt->dropSchema && te->desc &&
strcmp(te->desc, "DROP_GLOBAL") == 0)
continue;
best regards,
Ranier Vilela[4. text/x-diff; simplifies-checks-pg_basebackup_archiver.patch]...
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
Em ter., 10 de mar. de 2026 às 03:12, Japin Li <japinli@hotmail.com>
escreveu:
On Mon, 09 Mar 2026 at 10:07, Ranier Vilela <ranier.vf@gmail.com> wrote:
Hi.
Per Coverity.
Coverity has some alerts about the function *RestoreArchive*,
arguing that there are null variable dereferences.I haven't done in-depth research to determine if this is correct or not.
However, the code can be simplified to a single check that would silencenotifications and protect against this issue.
patch attached.
According to _allocAH(), te cannot be null.
AH->toc = pg_malloc0_object(TocEntry);
AH->toc->next = AH->toc;
AH->toc->prev = AH->toc;So we can safely remove its null check.
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index df8a69d3b79..137e5850f8e 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -767,7 +767,7 @@ RestoreArchive(Archive *AHX, bool append_data) continue; /* ignore if not to be dumped at all *//* Skip if no-tablespace is given. */ - if (ropt->noTablespace && te && te->desc && + if (ropt->noTablespace && te->desc && (strcmp(te->desc, "TABLESPACE") == 0)) continue;@@ -775,7 +775,7 @@ RestoreArchive(Archive *AHX, bool append_data) * Skip DROP DATABASE/ROLES/TABLESPACE if we didn't specify * --clean */ - if (!ropt->dropSchema && te && te->desc && + if (!ropt->dropSchema && te->desc && strcmp(te->desc, "DROP_GLOBAL") == 0) continue;
+1
Yeah, *te* It was already dereferenced before these checks.
Therefore, they are useless.
best regards
Ranier Vilela