Re: define bool in pgtypeslib_extern.h
On Fri, Nov 8, 2019 at 2:17 AM Tom Lane wrote: > > I wrote: > > I'm inclined to think that we need to make ecpglib.h's bool-related > > definitions exactly match c.h, which will mean that it has to pull in > > on most platforms, which will mean adding a control symbol > > for that to ecpg_config.h. I do not think we should export > > HAVE_STDBOOL_H and SIZEOF_BOOL there though; probably better to have > > configure make the choice and export something named like PG_USE_STDBOOL. > > Here's a proposed patch that does it like that. > > I'm of two minds about whether to back-patch or not. This shouldn't > really change anything except on platforms where sizeof(_Bool) isn't > one. We have some reason to think that nobody is actually using > ecpg on such platforms :-(, because if they were, they'd likely have > complained about breakage. > Yeah, this is a valid point, but I think this would have caused breakage only after d26a810eb which is a recent change. If that is right, then I am not sure such an assumption is safe. Also, we have already backpatched the probes.d change, so it seems reasonable to make this change and keep the bool definition consistent in code. OTOH, I think there is no harm in making this change for HEAD and if later we face any complaint, we can backpatch it. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: define bool in pgtypeslib_extern.h
I wrote: > I'm inclined to think that we need to make ecpglib.h's bool-related > definitions exactly match c.h, which will mean that it has to pull in > on most platforms, which will mean adding a control symbol > for that to ecpg_config.h. I do not think we should export > HAVE_STDBOOL_H and SIZEOF_BOOL there though; probably better to have > configure make the choice and export something named like PG_USE_STDBOOL. Here's a proposed patch that does it like that. I'm of two minds about whether to back-patch or not. This shouldn't really change anything except on platforms where sizeof(_Bool) isn't one. We have some reason to think that nobody is actually using ecpg on such platforms :-(, because if they were, they'd likely have complained about breakage. So maybe we should just put this in HEAD and be done. regards, tom lane diff --git a/configure b/configure index 7312bd7..21ee193 100755 --- a/configure +++ b/configure @@ -14729,6 +14729,12 @@ _ACEOF +if test "$ac_cv_header_stdbool_h" = yes -a "$ac_cv_sizeof_bool" = 1; then + +$as_echo "#define PG_USE_STDBOOL 1" >>confdefs.h + +fi + ## ## Functions, global variables diff --git a/configure.in b/configure.in index ea83d92..d128e59 100644 --- a/configure.in +++ b/configure.in @@ -1590,6 +1590,13 @@ AC_CHECK_SIZEOF([bool], [], #include #endif]) +dnl We use if we have it and it declares type bool as having +dnl size 1. Otherwise, c.h will fall back to declaring bool as unsigned char. +if test "$ac_cv_header_stdbool_h" = yes -a "$ac_cv_sizeof_bool" = 1; then + AC_DEFINE([PG_USE_STDBOOL], 1, +[Define to 1 to use to define type bool.]) +fi + ## ## Functions, global variables diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c index be68478..c8d2cef 100644 --- a/src/backend/utils/fmgr/dfmgr.c +++ b/src/backend/utils/fmgr/dfmgr.c @@ -23,7 +23,7 @@ * On macOS, insists on including . If we're not * using stdbool, undef bool to undo the damage. */ -#ifndef USE_STDBOOL +#ifndef PG_USE_STDBOOL #ifdef bool #undef bool #endif diff --git a/src/include/c.h b/src/include/c.h index c95acd3..5f800a3 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -288,20 +288,21 @@ * bool * Boolean value, either true or false. * - * Use stdbool.h if available and its bool has size 1. That's useful for + * We use stdbool.h if available and its bool has size 1. That's useful for * better compiler and debugger output and for compatibility with third-party * libraries. But PostgreSQL currently cannot deal with bool of other sizes; * there are static assertions around the code to prevent that. * * For C++ compilers, we assume the compiler has a compatible built-in * definition of bool. + * + * Note: this stanza also appears in src/interfaces/ecpg/include/ecpglib.h. */ #ifndef __cplusplus -#if defined(HAVE_STDBOOL_H) && SIZEOF_BOOL == 1 +#ifdef PG_USE_STDBOOL #include -#define USE_STDBOOL 1 #else #ifndef bool diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 2bf5060..249161c 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -823,6 +823,9 @@ /* Define to best printf format archetype, usually gnu_printf if available. */ #undef PG_PRINTF_ATTRIBUTE +/* Define to 1 to use to define type bool. */ +#undef PG_USE_STDBOOL + /* PostgreSQL version as a string */ #undef PG_VERSION diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32 index 6b67fb0..6c98ef4 100644 --- a/src/include/pg_config.h.win32 +++ b/src/include/pg_config.h.win32 @@ -624,6 +624,9 @@ (--with-krb-srvnam=NAME) */ #define PG_KRB_SRVNAM "postgres" +/* Define to 1 to use to define type bool. */ +#define PG_USE_STDBOOL 1 + /* A string containing the version number, platform, and C compiler */ #define PG_VERSION_STR "Uninitialized version string (win32)" diff --git a/src/interfaces/ecpg/include/ecpg_config.h.in b/src/interfaces/ecpg/include/ecpg_config.h.in index c185561..17e93c4 100644 --- a/src/interfaces/ecpg/include/ecpg_config.h.in +++ b/src/interfaces/ecpg/include/ecpg_config.h.in @@ -10,6 +10,9 @@ /* Define to 1 if `long long int' works and is 64 bits. */ #undef HAVE_LONG_LONG_INT_64 +/* Define to 1 to use to define type bool. */ +#undef PG_USE_STDBOOL + /* Define to 1 to build client libraries as thread-safe code. *(--enable-thread-safety) */ #undef ENABLE_THREAD_SAFETY diff --git a/src/interfaces/ecpg/include/ecpglib.h b/src/interfaces/ecpg/include/ecpglib.h index de9c76a..5cb31e2 100644 --- a/src/interfaces/ecpg/include/ecpglib.h +++ b/src/interfaces/ecpg/include/ecpglib.h @@ -1,6 +1,6 @@ /* - * this is a small part of c.h since we don't want to leak all postgres - * definitions into ecpg programs + * Client-visible declarations for ecpglib + * * src/interfaces/ecpg/include/ecpglib.h */ @@ -8,30 +8,36 @@ #define _ECPGLIB_H #include "libpq-fe.h" +#include "ecpg_config.h" #include
Re: define bool in pgtypeslib_extern.h
Andrew Gierth writes:
> I'm wondering whether we should actually go the opposite way and say
> that c.h's "bool" definition should be backend only, and that in
> frontend code we should define a PG_bool type or something of that ilk
> for when we want "PG's 1-byte bool" and otherwise let the platform
> define "bool" however it wants.
> And we certainly shouldn't be defining "bool" in something that's going
> to be included in the user's code the way that ecpglib.h is.
I experimented with doing things that way, and ended up with the attached
draft patch. It basically gets ecpglib.h out of the business of declaring
any bool-related stuff at all, instead insisting that client code include
or otherwise declare bool for itself. The function
declarations that were previously relying on "bool" now use the "pqbool"
typedef that libpq-fe.h was already exporting. Per discussion, that's
not an ABI break, even on platforms where sizeof(_Bool) > 1, because
the actual underlying library functions are certainly expecting to take
or return a value of size 1.
While this seems like a generally cleaner place to be, I'm a bit concerned
about a number of aspects:
* This will of course be an API break for clients, which might not've
included before.
* On platforms where sizeof(_Bool) > 1, it's far from clear to me that
ECPG will interface correctly with client code that is treating bool
as _Bool. There are some places that seem to be prepared for bool
client variables to be either sizeof(char) or sizeof(int), for example
ecpg_store_input(), but there are a fair number of other places that
seem to assume that sizeof(bool) is relevant, which it won't be.
The ECPG regression tests do pass for me on a PPC Mac, but I wonder
how much that proves.
* The "sql/dyntest.pgc" test declares BOOLVAR as "char" and then does
exec sql var BOOLVAR is bool;
It's not clear to me what the implications of that statement are
(and our manual is no help), but looking at the generated code,
it seems like this causes ecpg to believe that the size of the
variable is sizeof(bool). So that looks like buffer overrun
trouble waiting to happen. I changed the variable declaration to
"bool" in the attached, but I wonder what's supposed to be getting
tested there.
On the whole I'm not finding this an attractive way to proceed
compared to the other approach I sketched. It will certainly
cause some clients to have compile failures, and I'm at best
queasy about whether it will really work on platforms where
sizeof(_Bool) > 1. I think we're better off to go with the
other approach of making ecpglib.h export what we think the
correct definition of bool is. For most people that will
end up being , which I think will be unsurprising.
regards, tom lane
PS: another issue this fixes, which I think we ought to fix and back-patch
regardless of what we decide about bool, is it moves the declaration for
ecpg_gettext() out of ecpglib.h and into the private header
ecpglib_extern.h. That function isn't meant for client access, the
declaration is wrong where it is because it is not inside extern "C",
and the declaration wouldn't even compile for clients because they
will not know what pg_attribute_format_arg() is. The only reason we've
not had complaints, I imagine, is that nobody's tried to compile client
code with ENABLE_NLS defined ... but that's already an intrusion on
client namespace.
diff --git a/doc/src/sgml/ecpg.sgml b/doc/src/sgml/ecpg.sgml
index 8fe2a90..7e3f9ff 100644
--- a/doc/src/sgml/ecpg.sgml
+++ b/doc/src/sgml/ecpg.sgml
@@ -944,7 +944,7 @@ do
boolean
- booldeclared in ecpglib.h if not native
+ boolshould be declared by including stdbool.h
diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c
index 1cb5211..f816f8b 100644
--- a/src/interfaces/ecpg/ecpglib/connect.c
+++ b/src/interfaces/ecpg/ecpglib/connect.c
@@ -161,7 +161,7 @@ ecpg_finish(struct connection *act)
ecpg_log("ecpg_finish: called an extra time\n");
}
-bool
+pqbool
ECPGsetcommit(int lineno, const char *mode, const char *connection_name)
{
struct connection *con = ecpg_get_connection(connection_name);
@@ -198,7 +198,7 @@ ECPGsetcommit(int lineno, const char *mode, const char *connection_name)
return true;
}
-bool
+pqbool
ECPGsetconn(int lineno, const char *connection_name)
{
struct connection *con = ecpg_get_connection(connection_name);
@@ -267,7 +267,7 @@ ECPGnoticeReceiver(void *arg, const PGresult *result)
}
/* this contains some quick hacks, needs to be cleaned up, but it works */
-bool
+pqbool
ECPGconnect(int lineno, int c, const char *name, const char *user, const char *passwd, const char *connection_name, int autocommit)
{
struct sqlca_t *sqlca = ECPGget_sqlca();
@@ -680,7 +680,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
return true;
}
-bool
+pqbool
ECPGdisconnect(int lineno, const char
Re: define bool in pgtypeslib_extern.h
Amit Kapila writes: > On Mon, Oct 28, 2019 at 11:27 PM Tom Lane wrote: >> A plausible alternative is to do >> >> -#define bool char >> +#define bool unsigned char >> >> which is correct on platforms where we don't use , >> and is at least no worse than now on those where we do. In >> practice, since we know sizeof(_Bool) == 1 on platforms where >> we use it, this is probably just fine for dtrace's purposes. > +1 for the second alternative as it will make it similar to c.h. Yeah, that's the minimum-risk alternative. I'll go make it so. regards, tom lane
Re: define bool in pgtypeslib_extern.h
On Mon, Oct 28, 2019 at 11:27 PM Tom Lane wrote:
>
> Amit Kapila writes:
> > On Sat, Oct 26, 2019 at 10:49 PM Tom Lane wrote:
> >> I'm inclined to think that we need to make ecpglib.h's bool-related
> >> definitions exactly match c.h, which will mean that it has to pull in
> >> on most platforms, which will mean adding a control symbol
> >> for that to ecpg_config.h. I do not think we should export
> >> HAVE_STDBOOL_H and SIZEOF_BOOL there though; probably better to have
> >> configure make the choice and export something named like PG_USE_STDBOOL.
>
> > This sounds reasonable to me, but we also might want to do something
> > for probes.d. To be clear, I am not immediately planning to write a
> > patch for this.
>
> As far as probes.d goes, it seems to work to do
>
> @@ -20,7 +20,7 @@
> #define BlockNumber unsigned int
> #define Oid unsigned int
> #define ForkNumber int
> -#define bool char
> +#define bool _Bool
>
> provider postgresql {
>
> although removing the macro altogether leads to compilation failures.
> I surmise that dtrace is trying to compile the generated code without
> any #include's, so that only compiler built-in types will do.
>
> (I tried this on macOS, FreeBSD, and NetBSD, to the extent of seeing
> whether a build with --enable-dtrace goes through. I don't know
> enough about dtrace to test the results easily, but I suppose that
> if it compiles then this is OK.)
>
> This would, of course, not work on any platform where we're not
> using , but I doubt that the set of platforms where
> dtrace works includes any such.
>
> A plausible alternative is to do
>
> -#define bool char
> +#define bool unsigned char
>
> which is correct on platforms where we don't use ,
> and is at least no worse than now on those where we do. In
> practice, since we know sizeof(_Bool) == 1 on platforms where
> we use it, this is probably just fine for dtrace's purposes.
>
> Anyone have a preference?
>
+1 for the second alternative as it will make it similar to c.h.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Re: define bool in pgtypeslib_extern.h
Amit Kapila writes:
> On Sat, Oct 26, 2019 at 10:49 PM Tom Lane wrote:
>> I'm inclined to think that we need to make ecpglib.h's bool-related
>> definitions exactly match c.h, which will mean that it has to pull in
>> on most platforms, which will mean adding a control symbol
>> for that to ecpg_config.h. I do not think we should export
>> HAVE_STDBOOL_H and SIZEOF_BOOL there though; probably better to have
>> configure make the choice and export something named like PG_USE_STDBOOL.
> This sounds reasonable to me, but we also might want to do something
> for probes.d. To be clear, I am not immediately planning to write a
> patch for this.
As far as probes.d goes, it seems to work to do
@@ -20,7 +20,7 @@
#define BlockNumber unsigned int
#define Oid unsigned int
#define ForkNumber int
-#define bool char
+#define bool _Bool
provider postgresql {
although removing the macro altogether leads to compilation failures.
I surmise that dtrace is trying to compile the generated code without
any #include's, so that only compiler built-in types will do.
(I tried this on macOS, FreeBSD, and NetBSD, to the extent of seeing
whether a build with --enable-dtrace goes through. I don't know
enough about dtrace to test the results easily, but I suppose that
if it compiles then this is OK.)
This would, of course, not work on any platform where we're not
using , but I doubt that the set of platforms where
dtrace works includes any such.
A plausible alternative is to do
-#define bool char
+#define bool unsigned char
which is correct on platforms where we don't use ,
and is at least no worse than now on those where we do. In
practice, since we know sizeof(_Bool) == 1 on platforms where
we use it, this is probably just fine for dtrace's purposes.
Anyone have a preference?
regards, tom lane
Re: define bool in pgtypeslib_extern.h
On Sat, Oct 26, 2019 at 10:49 PM Tom Lane wrote: > > I'm inclined to think that we need to make ecpglib.h's bool-related > definitions exactly match c.h, which will mean that it has to pull in > on most platforms, which will mean adding a control symbol > for that to ecpg_config.h. I do not think we should export > HAVE_STDBOOL_H and SIZEOF_BOOL there though; probably better to have > configure make the choice and export something named like PG_USE_STDBOOL. > This sounds reasonable to me, but we also might want to do something for probes.d. To be clear, I am not immediately planning to write a patch for this. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: define bool in pgtypeslib_extern.h
Andrew Gierth writes: > "Tom" == Tom Lane writes: > Tom> I'm inclined to think that we need to make ecpglib.h's > Tom> bool-related definitions exactly match c.h, > I'm wondering whether we should actually go the opposite way and say > that c.h's "bool" definition should be backend only, and that in > frontend code we should define a PG_bool type or something of that ilk > for when we want "PG's 1-byte bool" and otherwise let the platform > define "bool" however it wants. > And we certainly shouldn't be defining "bool" in something that's going > to be included in the user's code the way that ecpglib.h is. The trouble here is the hazard of creating an ABI break, if we modify ecpglib.h in a way that causes its "bool" references to be interpreted differently than they were before. I don't think we want that (although I suspect we have inadvertently caused ABI breaks already on platforms where this matters). In practice, since v11 on every modern platform, the exported ecpglib functions have supposed that "bool" is _Bool, because they were compiled in files that included c.h before ecpglib.h. I assert furthermore that clients might well have included before ecpglib.h and thereby been fully compatible with that. If we start having ecpglib.h include itself, we'll just be eliminating a minor header inclusion order hazard. It's also rather hard to argue that including automatically is really likely to break anything that was including ecpglib.h already, since that file was already usurping those symbols. Except on platforms where sizeof(_Bool) isn't 1, but things are already pretty darn broken there. I think it's possible to construct a counterexample that will fail for *anything* we can do here. I'm not inclined to uglify things like mad to reduce the problem space from 0.1% to 0.01% of use-cases, or whatever the numbers would be in practice. regards, tom lane
Re: define bool in pgtypeslib_extern.h
> "Tom" == Tom Lane writes: Tom> On closer inspection, it seems to be just blind luck. For example, Tom> if I rearrange the inclusion order in a file using ecpglib.h: Ugh. Tom> I'm inclined to think that we need to make ecpglib.h's Tom> bool-related definitions exactly match c.h, I'm wondering whether we should actually go the opposite way and say that c.h's "bool" definition should be backend only, and that in frontend code we should define a PG_bool type or something of that ilk for when we want "PG's 1-byte bool" and otherwise let the platform define "bool" however it wants. And we certainly shouldn't be defining "bool" in something that's going to be included in the user's code the way that ecpglib.h is. -- Andrew.
Re: define bool in pgtypeslib_extern.h
Amit Kapila writes: > On Fri, Oct 25, 2019 at 9:55 PM Tom Lane wrote: >> However, we're not out of the woods, because lookee here in >> ecpglib.h: >> #ifndef bool >> #define bool char >> #endif /* ndef bool */ >> I'm more than slightly surprised that we haven't already seen >> problems due to this conflicting with d26a810eb. > I think it is because it never gets any imports from c.h. On closer inspection, it seems to be just blind luck. For example, if I rearrange the inclusion order in a file using ecpglib.h: diff --git a/src/interfaces/ecpg/ecpglib/data.c b/src/interfaces/ecpg/ecpglib/data.c index 7d2a78a..09944ff 100644 --- a/src/interfaces/ecpg/ecpglib/data.c +++ b/src/interfaces/ecpg/ecpglib/data.c @@ -6,8 +6,8 @@ #include #include "ecpgerrno.h" -#include "ecpglib.h" #include "ecpglib_extern.h" +#include "ecpglib.h" #include "ecpgtype.h" #include "pgtypes_date.h" #include "pgtypes_interval.h" then on a PPC Mac I get data.c:210: error: conflicting types for 'ecpg_get_data' ecpglib_extern.h:167: error: previous declaration of 'ecpg_get_data' was here It's exactly the same problem as we saw with pgtypeslib_extern.h: header ordering changes affect the meaning of uses of bool, and that's just not acceptable. In this case it's even worse because we're mucking with type definitions in a user-visible header. I'm surprised we've not gotten bug reports about that. Maybe all ECPG users include before they include ecpglib.h, but that doesn't exactly make things worry-free either, because code doing that will think that these functions return _Bool, when the compiled library possibly thinks differently. Probably the only thing saving us is that sizeof(_Bool) is 1 on just about every platform in common use nowadays. I'm inclined to think that we need to make ecpglib.h's bool-related definitions exactly match c.h, which will mean that it has to pull in on most platforms, which will mean adding a control symbol for that to ecpg_config.h. I do not think we should export HAVE_STDBOOL_H and SIZEOF_BOOL there though; probably better to have configure make the choice and export something named like PG_USE_STDBOOL. regards, tom lane
Re: define bool in pgtypeslib_extern.h
On Fri, Oct 25, 2019 at 9:55 PM Tom Lane wrote: > > I wrote: > > Amit Kapila writes: > >> As suggested by Andrew Gierth [1], I think we can remove the define in > >> pgtypeslib_extern.h as it doesn't seem to be exposed. > > > Yeah, it's not good that that results in a header ordering dependency, > > and it doesn't seem like a good idea for pgtypeslib_extern.h to be > > messing with the issue at all. > > If you like, I can experiment with that locally on prairiedog's host > > before we make the buildfarm jump through hoops. > > I checked that that works and fixes the immediate problem, so I pushed > it. > Thank you. > However, we're not out of the woods, because lookee here in > ecpglib.h: > > #ifndef __cplusplus > #ifndef bool > #define bool char > #endif /* ndef bool */ > > #ifndef true > #define true((bool) 1) > #endif /* ndef true */ > #ifndef false > #define false ((bool) 0) > #endif /* ndef false */ > #endif /* not C++ */ > > #ifndef TRUE > #define TRUE1 > #endif /* TRUE */ > > #ifndef FALSE > #define FALSE 0 > #endif /* FALSE */ > > This stuff *is* exposed to client programs, so it's not clear how > painless it'd be to monkey around with it. And it is used, further > down in the same file, so we can't fix it just by deleting it. > Nor can we import c.h to get the "real" definition from that. > > I'm more than slightly surprised that we haven't already seen > problems due to this conflicting with d26a810eb. > I think it is because it never gets any imports from c.h. It instead uses postgres_ext.h. If we want to fix this, the simplest thing that comes to mind is to change the definition of bool in ecpglib.h and probes.d to match with c.h. These files contain exposed interfaces, so the change can impact clients, but not sure what else we can do here. I have also tried to think about moving bool definition to postgres_ext.h, but I think that won't be straightforward. OTOH, if you think that might be worth investigating, I can spend some more time to see if we can do that way. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: define bool in pgtypeslib_extern.h
I wrote: > I checked that that works and fixes the immediate problem, so I pushed > it. However, we're not out of the woods, because lookee here in > ecpglib.h: > ... Oh, and for extra fun, take a look in src/backend/utils/probes.d :-( regards, tom lane
Re: define bool in pgtypeslib_extern.h
I wrote: > Amit Kapila writes: >> As suggested by Andrew Gierth [1], I think we can remove the define in >> pgtypeslib_extern.h as it doesn't seem to be exposed. > Yeah, it's not good that that results in a header ordering dependency, > and it doesn't seem like a good idea for pgtypeslib_extern.h to be > messing with the issue at all. > If you like, I can experiment with that locally on prairiedog's host > before we make the buildfarm jump through hoops. I checked that that works and fixes the immediate problem, so I pushed it. However, we're not out of the woods, because lookee here in ecpglib.h: #ifndef __cplusplus #ifndef bool #define bool char #endif /* ndef bool */ #ifndef true #define true((bool) 1) #endif /* ndef true */ #ifndef false #define false ((bool) 0) #endif /* ndef false */ #endif /* not C++ */ #ifndef TRUE #define TRUE1 #endif /* TRUE */ #ifndef FALSE #define FALSE 0 #endif /* FALSE */ This stuff *is* exposed to client programs, so it's not clear how painless it'd be to monkey around with it. And it is used, further down in the same file, so we can't fix it just by deleting it. Nor can we import c.h to get the "real" definition from that. I'm more than slightly surprised that we haven't already seen problems due to this conflicting with d26a810eb. I've not bothered to run to ground exactly why not, though. Thoughts? regards, tom lane
Re: define bool in pgtypeslib_extern.h
Amit Kapila writes: > As suggested by Andrew Gierth [1], I think we can remove the define in > pgtypeslib_extern.h as it doesn't seem to be exposed. Yeah, it's not good that that results in a header ordering dependency, and it doesn't seem like a good idea for pgtypeslib_extern.h to be messing with the issue at all. If you like, I can experiment with that locally on prairiedog's host before we make the buildfarm jump through hoops. regards, tom lane
define bool in pgtypeslib_extern.h
Today, I committed a patch (dddf4cdc) to reorder some of the file header inclusions and buildfarm members prairiedog and locust failed as a result of that. The reason turns out to be that we have defined a bool in pgtypeslib_extern.h and that definition is different from what we define in c.h. c.h defines it as: #ifndef bool typedef unsigned char bool; #endif pgtypeslib_extern.h defines it as: #ifndef bool #define bool char #endif Prior to dddf4cdc, pgtypeslib_extern.h was included as a first header before any usage of bool, but commit moves it after dt.h in file dt_common.c. Now, it seems like dt.h was using a version of bool as defined in c.h and dt_common.c uses as defined by pgtypeslib_extern.h which leads to some compilation errors as below: dt_common.c:672: error: conflicting types for 'EncodeDateOnly' dt.h:321: error: previous declaration of 'EncodeDateOnly' was here dt_common.c:756: error: conflicting types for 'EncodeDateTime' dt.h:316: error: previous declaration of 'EncodeDateTime' was here dt_common.c:1783: error: conflicting types for 'DecodeDateTime' dt.h:324: error: previous declaration of 'DecodeDateTime' was here make[4]: *** [dt_common.o] Error 1 As suggested by Andrew Gierth [1], I think we can remove the define in pgtypeslib_extern.h as it doesn't seem to be exposed. Thoughts? Note - For the time being, I have changed the order of file inclusions (c114229ca2) in dt_common.c as it was before so that the buildfarm becomes green again. [1] - https://www.postgresql.org/message-id/87h83xmg4m.fsf%40news-spur.riddles.org.uk -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
