Thread: BUG #15539: Deadcode in OpenTableList

BUG #15539: Deadcode in OpenTableList

From
PG Bug reporting form
Date:
The following bug has been logged on the website:

Bug reference:      15539
Logged by:          Pan Bian
Email address:      bianpan2016@163.com
PostgreSQL version: 11.1
Operating system:   Linux
Description:

File: src/backend/commands/publicationcmds.c
Function: OpenTableList
Issue details:
The function OpenTableList performs the condition check " if
(list_member_oid(relids, childrelid))" twice. The second condition will
always evaluate to FALSE. As a result, the code (i.e., heap_close) on the
true branch of the second condition check is deadcode.

For your convenience, I copy-and-paste related code as follows.

static List *
OpenTableList(List *tables)
{
    ...
    foreach(lc, tables)
    {
        ...
        if (recurse)
        {
            ...
            foreach(child, children)
            {
                Oid         childrelid = lfirst_oid(child);

                if (list_member_oid(relids, childrelid))   //### first
check
                    continue;

                /*  
                 * Skip duplicates if user specified both parent and child
                 * tables.
                 */
                if (list_member_oid(relids, childrelid))  //### second
check, always be false
                {  //### deadcode
                    heap_close(rel, ShareUpdateExclusiveLock);
                    continue;
                }

                /* find_all_inheritors already got lock */
                rel = heap_open(childrelid, NoLock);
                rels = lappend(rels, rel);
                relids = lappend_oid(relids, childrelid);
            }
        }
    }   

    list_free(relids);

    return rels;
}

Thank you,
Pan Bian


Re: BUG #15539: Deadcode in OpenTableList

From
Tom Lane
Date:
=?utf-8?q?PG_Bug_reporting_form?= <noreply@postgresql.org> writes:
> The function OpenTableList performs the condition check " if
> (list_member_oid(relids, childrelid))" twice. The second condition will
> always evaluate to FALSE. As a result, the code (i.e., heap_close) on the
> true branch of the second condition check is deadcode.

Yeah, you're right.  I wonder whether we should've expected Coverity
to notice that.

Fix pushed, thanks for the report!

            regards, tom lane