Thread

1. Simplifies checks (src/bin/pg_dump/pg_backup_archiver.c)

   Ranier Vilela <ranier.vf@gmail.com> — 2026-03-09T13:07:19Z

   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
   

2. Re: Simplifies checks (src/bin/pg_dump/pg_backup_archiver.c)

   Daniel Gustafsson <daniel@yesql.se> — 2026-03-09T13:25:18Z

   > 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
   
   
   
   
   

3. Re: Simplifies checks (src/bin/pg_dump/pg_backup_archiver.c)

   Japin Li <japinli@hotmail.com> — 2026-03-10T06:12:23Z

   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.
   
   
   
   

4. Re: Simplifies checks (src/bin/pg_dump/pg_backup_archiver.c)

   Ranier Vilela <ranier.vf@gmail.com> — 2026-03-10T11:09:46Z

   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 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;
   >
   +1
   
   Yeah, *te* It was already dereferenced before these checks.
   Therefore, they are useless.
   
   best regards
   Ranier Vilela