Thread: Warnings triggered by recent includefile cleanups
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
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
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
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
> 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: ---------------------------------------------------------------------------
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