Re: taking stdbool.h into use

On Wed, Aug 16, 2017 at 4:36 PM, Peter Eisentraut
<peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> Some not so long time ago, it was discussed to look into taking
> stdbool.h into use. The reason was that third-party libraries (perl?,
> ldap?, postgis?) are increasingly doing so, and having incompatible
> definitions of bool could/does create a mess.
>
> Here is a patch set that aims to accomplish that. On the way there, it
> cleans up various loose and weird uses of bool and proposes a way to
> ensure that the system catalog structs get a 1-byte bool even if the
> "standard" bool is not.
>
> I have done a fair amount of testing on this, including a hand-rigged
> setup where _Bool is not 1 byte. But obviously, more and wider testing
> would be very useful.

Getting out of the way of C99 "bool" makes sense. It is old enough to
vote. On my system the tests pass at top level and tcl, plperl,
plpython after each patch is applied, when configured with:

--enable-debug --enable-depend --enable-cassert --with-icu --with-python
--with-perl --with-tcl --with-icu --with-ldap

However my system has sizeof(bool) == 1 and so do all the systems I
have access to (x86 + POWER). Where can we find a computer with
sizeof(bool) == 4? According to the intertubes OSX on POWER and
Windows 32 bit systems had that in ancient prehistory but they don't
now.

> 0001-Fix-bool-int-type-confusion.patch

Looks good.

> 0002-Change-TRUE-FALSE-to-true-false.patch

Looks good. What about these?

src/backend/port/win32/crashdump.c: ExInfo.ClientPointers = FALSE;
src/backend/port/win32/signal.c: pgwin32_signal_event =
CreateEvent(NULL, TRUE, FALSE, NULL);
src/backend/port/win32/signal.c:
SleepEx(500, FALSE);
src/backend/port/win32/signal.c: return FALSE;
src/backend/port/win32/socket.c: waitevent =
CreateEvent(NULL, TRUE, FALSE, NULL);
src/backend/port/win32/socket.c: r =
WaitForMultipleObjectsEx(2, events, FALSE, 100, TRUE);
src/backend/port/win32/socket.c: r =
WaitForMultipleObjectsEx(2, events, FALSE, timeout, TRUE);
src/backend/port/win32/socket.c: r =
WaitForMultipleObjectsEx(numevents + 1, events, FALSE, timeoutval,
TRUE);
src/backend/port/win32/timer.c: r =
WaitForSingleObjectEx(timerCommArea.event, waittime, FALSE);
src/backend/port/win32/timer.c: timerCommArea.event =
CreateEvent(NULL, TRUE, FALSE, NULL);
src/backend/port/win32_sema.c: rc =
WaitForMultipleObjectsEx(2, wh, FALSE, INFINITE, TRUE);
src/backend/port/win32_shmem.c: hmap = OpenFileMapping(FILE_MAP_READ,
FALSE, szShareMem);
src/backend/storage/ipc/dsm_impl.c:
FALSE, /* do not inherit the name */
src/backend/storage/ipc/dsm_impl.c:
PostmasterHandle, &hmap, 0, FALSE,
src/backend/storage/ipc/dsm_impl.c:
NULL, NULL, 0, FALSE,
src/backend/storage/ipc/latch.c: latch->event =
CreateEvent(NULL, TRUE, FALSE, NULL);
src/backend/storage/ipc/latch.c: latch->event =
CreateEvent(&sa, TRUE, FALSE, NULL);
src/interfaces/ecpg/ecpglib/misc.c: return FALSE;
src/interfaces/ecpg/ecpglib/misc.c: return FALSE;
src/interfaces/ecpg/ecpglib/misc.c: mutex->handle
= CreateMutex(NULL, FALSE, NULL);
src/interfaces/ecpg/pgtypeslib/datetime.c: bool
EuroDates = FALSE;
src/interfaces/ecpg/pgtypeslib/datetime.c: bool
EuroDates = FALSE;
src/interfaces/ecpg/pgtypeslib/dt_common.c: int
bc = FALSE;
src/interfaces/ecpg/pgtypeslib/dt_common.c: int
is2digits = FALSE;
src/interfaces/ecpg/pgtypeslib/dt_common.c: int
haveTextMonth = FALSE;
src/interfaces/ecpg/pgtypeslib/dt_common.c: int
is2digits = FALSE;
src/interfaces/ecpg/pgtypeslib/dt_common.c: int
bc = FALSE;
src/interfaces/ecpg/pgtypeslib/interval.c: bool
is_before = FALSE;
src/interfaces/ecpg/pgtypeslib/interval.c: *is_zero = FALSE;
src/interfaces/ecpg/pgtypeslib/interval.c: *is_zero = FALSE;
src/interfaces/ecpg/pgtypeslib/interval.c: bool
is_before = FALSE;
src/interfaces/ecpg/pgtypeslib/interval.c:
is_zero = FALSE;
src/interfaces/ecpg/pgtypeslib/numeric.c: bool have_dp = FALSE;
src/interfaces/ecpg/pgtypeslib/timestamp.c: return FALSE;
src/interfaces/libpq/fe-secure.c: * The caller should say got_epipe =
FALSE if it is certain that it
src/pl/plperl/plperl.c:
eval_pv("PostgreSQL::InServer::SPI::bootstrap()", FALSE);
src/pl/plperl/plperl.c: eval_pv(PLC_TRUSTED, FALSE);
src/pl/plperl/plperl.c: eval_pv("my $a=chr(0x100); return $a =~
/\\xa9/i", FALSE);
src/pl/plperl/plperl.c: eval_pv(plperl_on_plperl_init, FALSE);
src/pl/plperl/plperl.c: eval_pv(plperl_on_plperlu_init, FALSE);
src/pl/plperl/plperl.c: SV **svp = av_fetch(av,
i, FALSE);
src/pl/plperl/plperl.c: while ((svp = av_fetch(rav, i,
FALSE)) != NULL)
src/pl/plperl/ppport.h:# define ERRSV
get_sv("@",FALSE)
src/pl/plperl/ppport.h: utilize(!(flags & PERL_LOADMOD_DENY),
start_subparse(FALSE, 0),
src/pl/plperl/ppport.h: start_subparse(FALSE, 0),
src/pl/plperl/ppport.h: SV *my_cxt_sv = get_sv(MY_CXT_KEY, FALSE)
src/pl/plperl/ppport.h: return FALSE;
src/pl/plperl/ppport.h: bool overflowed = FALSE;
src/pl/plperl/ppport.h: bool overflowed = FALSE;
src/pl/plperl/ppport.h: bool overflowed = FALSE;
src/pl/plpgsql/src/pl_comp.c: * if not recognized, fill in *word and
return FALSE.
src/port/kill.c: if ((prochandle =
OpenProcess(PROCESS_TERMINATE, FALSE, (DWORD) pid)) == NULL)
src/port/pgsleep.c: SleepEx((microsec < 500 ? 1 :
(microsec + 500) / 1000), FALSE);
src/test/regress/pg_regress.c: r =
WaitForMultipleObjects(tests_left, active_pids, FALSE, INFINITE);

> 0003-Remove-TRUE-and-FALSE.patch

Looks good. What about these?

src/interfaces/ecpg/include/ecpglib.h:#define FALSE 0
src/interfaces/ecpg/pgtypeslib/extern.h:#define FALSE 0

> 0004-Remove-BoolPtr-type.patch

Looks good.

> 0005-Make-casting-between-bool-and-GinTernaryValue-more-r.patch

- * For convenience, this is compatible with booleans. A boolean can be
+ * For convenience, this is compatible with bools. A bool can be

Maybe "compatible with bool" ?

+#elif SIZEOF_BOOL == 4
+typedef int GinTernaryValue;
+#else

Maybe int32? I understand that PostgreSQL doesn't actually work if
int isn't of that size, but still.

> 0006-Add-bool8-typedef-for-system-catalog-structs.patch

Make sense.

> 0007-Avoid-use-of-bool-in-thread_test.c.patch

Looks good.

> 0008-Use-stdbool.h-if-available.patch

I'm afraid this autoconf stuff is gibberish, and I can't personally
tell if it's the right gibberish so no opinion on this one.

--
Thomas Munro
http://www.enterprisedb.com