Thread: Warnings triggered by recent includefile cleanups

Warnings triggered by recent includefile cleanups

From
Tom Lane
Date:
For the last week or so I've been getting warnings like this:

gcc -c  -I../../../../src/include  -O2 -Wall -Wmissing-prototypes -Wmissing-declarations -g -O1 -MMD float.c -o
float.o
In file included from float.c:58:
/usr/include/values.h:27: warning: `MAXINT' redefined
/usr/local/lib/gcc-lib/hppa2.0-hp-hpux10.20/2.95.2/include/sys/param.h:46: warning: this is the location of the
previousdefinition
 

in half a dozen backend files.  On investigation I find that the cause
is this recent change:
-#ifdef HAVE_LIMITS_H#include <limits.h>
-#ifndef MAXINT
-#define MAXINT           INT_MAX
-#endif
-#else#ifdef HAVE_VALUES_H#include <values.h>
-#endif#endif

specifically the fact that the code now tries to include *both*
<limits.h> and <values.h> rather than only one.  Well, I'm here
to tell you that the two headers are not entirely compatible,
at least not on this platform (HPUX 10.20, obviously).

Checking the CVS logs, I see that 7.0 is our first release that tries
to include <values.h> at all, so we have little track experience with
that header and none with its possible conflicts with the ANSI-standard
headers.  The submitter of the patch that added it did not recommend
including it unconditionally, but only if <limits.h> is not available.
Looks like he knew what he was doing.

Does anyone object if I revert this code to the way it was?
        regards, tom lane


Re: Warnings triggered by recent includefile cleanups

From
Peter Eisentraut
Date:
Tom Lane writes:

> specifically the fact that the code now tries to include *both*
> <limits.h> and <values.h> rather than only one.  Well, I'm here
> to tell you that the two headers are not entirely compatible,
> at least not on this platform (HPUX 10.20, obviously).
> 
> Checking the CVS logs, I see that 7.0 is our first release that tries
> to include <values.h> at all, so we have little track experience with
> that header and none with its possible conflicts with the ANSI-standard
> headers.  The submitter of the patch that added it did not recommend
> including it unconditionally, but only if <limits.h> is not available.
> Looks like he knew what he was doing.
> 
> Does anyone object if I revert this code to the way it was?

Considering that evidence shows that limits.h must have been available on
all platforms at least since 6.5, in fact at least as long as the current
regex engine has existed, values.h could not possibly have been included
anywhere ever, so it's probably better to just remove it.


-- 
Peter Eisentraut                  Sernanders väg 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden



Re: Warnings triggered by recent includefile cleanups

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
>> Does anyone object if I revert this code to the way it was?

> Considering that evidence shows that limits.h must have been available on
> all platforms at least since 6.5, in fact at least as long as the current
> regex engine has existed, values.h could not possibly have been included
> anywhere ever, so it's probably better to just remove it.

Hmm, it does look like regex has included <limits.h> unconditionally
since day 1, doesn't it?  That sure suggests that this patch:

@@ -7,7 +7,7 @@ * * * IDENTIFICATION
- *       $Header: /home/projects/pgsql/cvsroot/pgsql/src/backend/utils/adt/float.c,v 1.47 1999/07/17 20:17:55 momjian
Exp$
 
+ *       $Header: /home/projects/pgsql/cvsroot/pgsql/src/backend/utils/adt/float.c,v 1.48 1999/09/21 20:58:25 momjian
Exp$ * *------------------------------------------------------------------------- */
 
@@ -55,6 +55,13 @@#include "postgres.h"#ifdef HAVE_LIMITS_H#include <limits.h>
+#ifndef MAXINT
+#define MAXINT           INT_MAX
+#endif
+#else
+#ifdef HAVE_VALUES_H
+#include <values.h>
+#endif#endif#include "fmgr.h"#include "utils/builtins.h"

was dead code when it was installed.  The CVS log saysvalues.h patch from  Alex Howansky
but I can't find anything from him in the mailing list archives.
(We seem to have lost the pgsql-patches archives, however, so if it
was just a patch that went by then there's no remaining doco.)

Bruce, does this ring a bell at all?  Unless someone can remember
why this change seemed like a good idea, I think I will take Peter's
advice...
        regards, tom lane


Re: Warnings triggered by recent includefile cleanups

From
Peter Eisentraut
Date:
Tom Lane writes:

> @@ -55,6 +55,13 @@
>  #include "postgres.h"
>  #ifdef HAVE_LIMITS_H
>  #include <limits.h>
> +#ifndef MAXINT
> +#define MAXINT           INT_MAX
> +#endif
> +#else
> +#ifdef HAVE_VALUES_H
> +#include <values.h>
> +#endif
>  #endif
>  #include "fmgr.h"
>  #include "utils/builtins.h"
> 
> was dead code when it was installed.  The CVS log says
>     values.h patch from  Alex Howansky

He claimed that the compilation failed for him in the files   src/backend/optimizer/path/costsize.c
src/backend/utils/adt/date.c  src/backend/utils/adt/float.c
 
because MAXINT was undefined (although neither date.c nor float.c used
MAXINT at all in 6.5).

This problem is gone and one should use INT_MAX anyway.


-- 
Peter Eisentraut                  Sernanders väg 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden



Re: Warnings triggered by recent includefile cleanups

From
Bruce Momjian
Date:
> Peter Eisentraut <peter_e@gmx.net> writes:
> >> Does anyone object if I revert this code to the way it was?
> 
> > Considering that evidence shows that limits.h must have been available on
> > all platforms at least since 6.5, in fact at least as long as the current
> > regex engine has existed, values.h could not possibly have been included
> > anywhere ever, so it's probably better to just remove it.
> 
> Hmm, it does look like regex has included <limits.h> unconditionally
> since day 1, doesn't it?  That sure suggests that this patch:
> 
> @@ -7,7 +7,7 @@
>   *
>   *
>   * IDENTIFICATION
> - *       $Header: /home/projects/pgsql/cvsroot/pgsql/src/backend/utils/adt/float.c,v 1.47 1999/07/17 20:17:55
momjianExp $
 
> + *       $Header: /home/projects/pgsql/cvsroot/pgsql/src/backend/utils/adt/float.c,v 1.48 1999/09/21 20:58:25
momjianExp $
 
>   *
>   *-------------------------------------------------------------------------
>   */
> @@ -55,6 +55,13 @@
>  #include "postgres.h"
>  #ifdef HAVE_LIMITS_H
>  #include <limits.h>
> +#ifndef MAXINT
> +#define MAXINT           INT_MAX
> +#endif
> +#else
> +#ifdef HAVE_VALUES_H
> +#include <values.h>
> +#endif
>  #endif
>  #include "fmgr.h"
>  #include "utils/builtins.h"
> 
> was dead code when it was installed.  The CVS log says
>     values.h patch from  Alex Howansky
> but I can't find anything from him in the mailing list archives.
> (We seem to have lost the pgsql-patches archives, however, so if it
> was just a patch that went by then there's no remaining doco.)
> 
> Bruce, does this ring a bell at all?  Unless someone can remember
> why this change seemed like a good idea, I think I will take Peter's
> advice...

I have:

---------------------------------------------------------------------------



Re: Warnings triggered by recent includefile cleanups

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> Bruce, does this ring a bell at all?  Unless someone can remember
>> why this change seemed like a good idea, I think I will take Peter's
>> advice...

> I have:

Hm.  Looks like the ifndef MAXINT was the part he actually cared about,
and that's now dead code since we don't use MAXINT anywhere anymore.
So I'll go ahead and simplify it down to just #include <limits.h>.
Thanks.
        regards, tom lane