Re: [HACKERS] Warnings in objectaddress.c

On Sun, Oct 1, 2017 at 10:33 AM, Дмитрий Воронин
<carriingfate92@yandex.ru> wrote:
> I'm building PostgreSQL 10 rc1 sources on Debian wheezy (gcc 4.7). I have those warnings:
>
> objectaddress.c: In function ‘get_object_address’:
> objectaddress.c:1646:10: warning: ‘typeoids[1]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> objectaddress.c:1578:8: note: ‘typeoids[1]’ was declared here
> objectaddress.c:1623:7: warning: ‘typenames[1]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> objectaddress.c:1577:14: note: ‘typenames[1]’ was declared here
> objectaddress.c:1646:10: warning: ‘typeoids[0]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> objectaddress.c:1578:8: note: ‘typeoids[0]’ was declared here
> objectaddress.c:1623:7: warning: ‘typenames[0]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> objectaddress.c:1577:14: note: ‘typenames[0]’ was declared here
>
> Those variables typeoids and typenames are arrays and are not initialized during definition.
>
> Hope this helps. Thank you!

In theory I think this isn't a problem because List *object should
always be a two-element list whose second element is itself a list of
at least 2 elements, but in practice this doesn't seem like very good
code, because if by chance that list isn't of the proper format, we'll
either crash (if the toplevel list is too short) or do syscache
lookups using undefined values (if the sub-list is too short).  The
latter possibility is, I believe, what prompts these warnings. Perhaps
we should apply some glorified version of this:

diff --git a/src/backend/catalog/objectaddress.c
b/src/backend/catalog/objectaddress.c
index c2ad7c675e..9c27ab9434 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -1579,6 +1579,9 @@ get_object_address_opf_member(ObjectType objtype,    int         membernum;    int         i;

+    if (list_length(object) < 2)
+        elog(ERROR, "fail");
+    /*     * The last element of the object list contains the strategy or procedure     * number.  We need to strip
thatout before getting the opclass/family 
@@ -1603,6 +1606,9 @@ get_object_address_opf_member(ObjectType objtype,            break;    }

+    if (i < 2)
+        elog(ERROR, "somebody screwed up");
+    switch (objtype)    {        case OBJECT_AMOP:

However, I'm not 100% sure that would be sufficient to suppress these
warnings, because the compiler has got to be smart enough to know that
elog() doesn't return and that i >= 2 is sufficient to guarantee that
everything is initialized.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers