I noticed the following compile warnings. Perhaps someone is interested
to know about them.

/usr/bin/flex -o'pgc.c' pgc.l
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline
-Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -g
-Wno-error -I./../include -I. -I../../../../src/include -D_GNU_SOURCE
-DMAJOR_VERSION=4 -DMINOR_VERSION=2 -DPATCHLEVEL=1 -c -o preproc.o
preproc.c -MMD
In file included from preproc.y:6606:
pgc.c: In function ‘yylex’:
pgc.c:1549: warning: label ‘find_rule’ defined but not used

_GNU_SOURCE -c -o mac.o mac.c -MMD
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline
-Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -g
-I../../../../src/include -D_GNU_SOURCE -c -o inet_net_ntop.o
inet_net_ntop.c -MMD
inet_net_ntop.c: In function ‘inet_cidr_ntop_ipv6’:
inet_net_ntop.c:292: warning: value computed is not used


---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Kris Jurka <books@ejurka.com> writes:
> Also I was testing a gcc 4.2 snapshot (20060419) and it has a whole lot of
> warnings stemming from heap_getattr's isnull check:
> aclchk.c:791: warning: the address of 'isNull', will always evaluate as
> 'true'

We need to lobby the gcc maintainers to not give warnings about valid
and perfectly reasonable code. I'm not thrilled with changing the API
of a key macro for no other reason than that gcc has started to complain
about it.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Gevik Babakhani wrote:
> I noticed the following compile warnings. Perhaps someone is interested
> to know about them.
>
> /usr/bin/flex -o'pgc.c' pgc.l
> gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline
> -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -g
> -Wno-error -I./../include -I. -I../../../../src/include -D_GNU_SOURCE
> -DMAJOR_VERSION=4 -DMINOR_VERSION=2 -DPATCHLEVEL=1 -c -o preproc.o
> preproc.c -MMD
> In file included from preproc.y:6606:
> pgc.c: In function ?yylex?:
> pgc.c:1549: warning: label ?find_rule? defined but not used

This is a standard warning generated by bison and we can't get rid of
it.

> _GNU_SOURCE -c -o mac.o mac.c -MMD
> gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline
> -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -g
> -I../../../../src/include -D_GNU_SOURCE -c -o inet_net_ntop.o
> inet_net_ntop.c -MMD
> inet_net_ntop.c: In function ?inet_cidr_ntop_ipv6?:
> inet_net_ntop.c:292: warning: value computed is not used

I have fixed this by adding a (void) cast, patch attached.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Index: src/backend/utils/adt/inet_net_ntop.c
================================================== =================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/inet_net_ntop.c,v
retrieving revision 1.21
diff -c -c -r1.21 inet_net_ntop.c
*** src/backend/utils/adt/inet_net_ntop.c 15 Oct 2005 02:49:28 -0000 1.21
--- src/backend/utils/adt/inet_net_ntop.c 24 Apr 2006 19:49:04 -0000
***************
*** 289,295 ****
}
}
/* Format CIDR /width. */
! SPRINTF((cp, "/%u", bits));
if (strlen(outbuf) + 1 > size)
goto emsgsize;
strcpy(dst, outbuf);
--- 289,295 ----
}
}
/* Format CIDR /width. */
! (void) SPRINTF((cp, "/%u", bits));
if (strlen(outbuf) + 1 > size)
goto emsgsize;
strcpy(dst, outbuf);


---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Tom Lane wrote:
> Kris Jurka <books@ejurka.com> writes:
> > Also I was testing a gcc 4.2 snapshot (20060419) and it has a whole lot of
> > warnings stemming from heap_getattr's isnull check:
> > aclchk.c:791: warning: the address of 'isNull', will always evaluate as
> > 'true'
>
> We need to lobby the gcc maintainers to not give warnings about valid
> and perfectly reasonable code. I'm not thrilled with changing the API
> of a key macro for no other reason than that gcc has started to complain
> about it.

Right. The issue is that when I originally transfered that function to
a macro, the isnull parameter was optionally 0/NULL. While our code
doesn't use that ability, I see no reason to remove it.

I understand why it is complaining because you are really doing if
(&var), but it is a macro, so it can be used in other circumstances as
well.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

On Mon, Apr 24, 2006 at 01:16:04PM -0500, Kris Jurka wrote:
>
>
> On Mon, 24 Apr 2006, Gevik Babakhani wrote:
>
> >I noticed the following compile warnings. Perhaps someone is interested
> >to know about them.
>
> Also I was testing a gcc 4.2 snapshot (20060419) and it has a whole lot of
> warnings stemming from heap_getattr's isnull check:
>
> aclchk.c:791: warning: the address of 'isNull', will always evaluate as
> 'true'

Perhaps someone could check if changing the test explicitly check
against NULL:

> ((attnum) > (int) (tup)->t_data->t_natts) ? \
> ( \
> (((isnull) != NULL)? (*(isnull) = true) : (dummyret)NULL), \
> (Datum)NULL \

removes the warning. It seems silly for the GCC people to add warnings
for this kind of stuff without a simple way to bypass it...
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFETTcmIB7bNG8LQkwRAi4cAJsH35xXLjrXKbyzr487Yj w6JdUEGQCcCYMG
83lPpNyxilc6L3gToSUPv/c=
=hNjR
-----END PGP SIGNATURE-----

On Mon, 24 Apr 2006, Martijn van Oosterhout wrote:

> Perhaps someone could check if changing the test explicitly check
> against NULL:
>
>> ((attnum) > (int) (tup)->t_data->t_natts) ? \
>> ( \
>> (((isnull) != NULL)? (*(isnull) = true) : (dummyret)NULL), \
>> (Datum)NULL \
>
> removes the warning. It seems silly for the GCC people to add warnings
> for this kind of stuff without a simple way to bypass it...

Yes, this coding removes the warning.

Kris Jurka


---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Kris Jurka <books@ejurka.com> writes:
> On Mon, 24 Apr 2006, Martijn van Oosterhout wrote:
>> Perhaps someone could check if changing the test explicitly check
>> against NULL:
>>
>> ((attnum) > (int) (tup)->t_data->t_natts) ? \
>> ( \
>> (((isnull) != NULL)? (*(isnull) = true) : (dummyret)NULL), \
>> (Datum)NULL \
>>
>> removes the warning. It seems silly for the GCC people to add warnings
>> for this kind of stuff without a simple way to bypass it...

> Yes, this coding removes the warning.

Oh, good, that seems like a reasonable change to make (it's arguably
more clear than the original anyway).

Is this the only place where the warning shows up? ISTM there's quite
a lot of code that uses "if (ptr)" for a NULL-ness check.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

On Mon, Apr 24, 2006 at 05:39:30PM -0400, Tom Lane wrote:
> > Yes, this coding removes the warning.
>
> Oh, good, that seems like a reasonable change to make (it's arguably
> more clear than the original anyway).
>
> Is this the only place where the warning shows up? ISTM there's quite
> a lot of code that uses "if (ptr)" for a NULL-ness check.

But very little code of the form "if (&local_var)" which is always true
and what causes the problem here. I think this is just a variation on
the compiler test for "if (i=1)" which is also always true but probably
a bug. That warning you avoid with an extra set of parenthesis.

I'd be surprised if there were many other issues here, only complex
macros are likely to cause this one.

Have a nice day,
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFETUmcIB7bNG8LQkwRAq+7AKCE2RDsLWbos8hoH8If9D YqDlgFTgCZAdPx
p19xYytELKPmH3c9uwSbyZg=
=w1/f
-----END PGP SIGNATURE-----

Kris Jurka wrote:
>
>
> On Mon, 24 Apr 2006, Martijn van Oosterhout wrote:
>
> > Perhaps someone could check if changing the test explicitly check
> > against NULL:
> >
> >> ((attnum) > (int) (tup)->t_data->t_natts) ? \
> >> ( \
> >> (((isnull) != NULL)? (*(isnull) = true) : (dummyret)NULL), \
> >> (Datum)NULL \
> >
> > removes the warning. It seems silly for the GCC people to add warnings
> > for this kind of stuff without a simple way to bypass it...
>
> Yes, this coding removes the warning.

Great, fix attached and applied.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Index: src/include/access/heapam.h
================================================== =================
RCS file: /cvsroot/pgsql/src/include/access/heapam.h,v
retrieving revision 1.107
diff -c -c -r1.107 heapam.h
*** src/include/access/heapam.h 24 Mar 2006 04:32:13 -0000 1.107
--- src/include/access/heapam.h 24 Apr 2006 22:03:14 -0000
***************
*** 100,106 ****
( \
((attnum) > (int) (tup)->t_data->t_natts) ? \
( \
! ((isnull) ? (*(isnull) = true) : (dummyret)NULL), \
(Datum)NULL \
) \
: \
--- 100,106 ----
( \
((attnum) > (int) (tup)->t_data->t_natts) ? \
( \
! ((isnull) != NULL) ? (*(isnull) = true) : (dummyret)NULL), \
(Datum)NULL \
) \
: \


---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

On Mon, 24 Apr 2006, Bruce Momjian wrote:

> Great, fix attached and applied.
>

You also need to change lines 48 and 64 of heapam.h to use the same
coding.

Kris Jurka

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings