Re: [HACKERS] Warnings in objectaddress.c

[Robert Haas]

On Tue, Oct 3, 2017 at 9:41 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> Perhaps we should apply some glorified version of this:
>
>> +if (list_length(object) < 2)
>> +elog(ERROR, "fail");
>
>> 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.
>
> I'm betting it wouldn't help.  I was considering something along the line
> of unrolling the loop:
>
> Assert(list_length(object) == 2);
>
> assign typenames[0] and typeoids[0] from linitial(object)
>
> assign typenames[1] and typeoids[1] from lsecond(object)
>
> This would involve duplicating the loop body, but that's only 3 lines,
> so I think it wouldn't even net out as more code.

Yeah, that's an idea, too.

-- 
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

Re: [HACKERS] Warnings in objectaddress.c

[Tom Lane]

Robert Haas  writes:
> Perhaps we should apply some glorified version of this:

> +if (list_length(object) < 2)
> +elog(ERROR, "fail");

> 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.

I'm betting it wouldn't help.  I was considering something along the line
of unrolling the loop:

Assert(list_length(object) == 2);

assign typenames[0] and typeoids[0] from linitial(object)

assign typenames[1] and typeoids[1] from lsecond(object)

This would involve duplicating the loop body, but that's only 3 lines,
so I think it wouldn't even net out as more code.

regards, tom lane


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

Re: [HACKERS] Warnings in objectaddress.c

[Robert Haas]

On Sun, Oct 1, 2017 at 10:33 AM, Дмитрий Воронин
 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 that out 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

[HACKERS] Warnings in objectaddress.c

[Дмитрий Воронин]

Hello, hackers!

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!

-- 
Best regargs, Dmitry Voronin


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