Re: Preliminary results for proposed new pgindent implementation

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Piotr Stefaniak <postgres(at)piotr-stefaniak(dot)me>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, Andres Freund <andres(at)anarazel(dot)de>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Stephen Frost" <sfrost(at)snowman(dot)net>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Preliminary results for proposed new pgindent implementation
Date: 2017-06-17 19:55:11
Message-ID: 11234.1497729311@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Piotr Stefaniak <postgres(at)piotr-stefaniak(dot)me> writes:
>> There are also the "portability fixes" and they're the main problem.

> Fair enough.

I spent some time looking into this. I reverted your commits
198457848ae5c86bec3336a9437dd5aa30f480c2 (Replace err.h functions with
standard C equivalents) and fb10acb040b90bdcbad09defd303363db29257d1
(Remove inclusion of sys/cdefs.h) locally and tried to build without
those. I've successfully worked around the err.h change by adding
cut-down versions of FreeBSD 11's err.h and err.c to the fileset
(see attached). However, it's proving impossible to work around having
"#include <sys/cdefs.h>" as the first live code in the files. I thought
maybe we could provide a dummy cdefs.h file, but that breaks things on
platforms where cdefs.h is a real thing and is relied on by other system
headers --- which includes both Linux and BSD. It seems we would have
to have something like #ifdef HAVE_SYS_CDEFS_H, but that is already a
departure from FreeBSD practice.

So what I'm currently thinking is that we have to diverge from the
FreeBSD sources to the extent of removing #include <sys/cdefs.h>
and the __FBSDID() calls, and instead inserting #include "c.h" to
pick up PG's own portability definitions. The thing that forced me
into the latter is that there seems no way to avoid compiler warnings
if we don't decorate the declarations of err() and errx() with noreturn
and printf-format attributes --- and we need c.h to provide portable
ways of writing those. But there are probably other portability things
that we'll need c.h for, anyway, especially if we want to make it work
on Windows. So I'm thinking this is a small and easily maintainable
difference from the upstream FreeBSD files.

When I inserted #include "c.h", I got duplicate-macro-definition warnings
about "true" and "false", so I would ask you to add this:

--- freebsd_indent/indent_globs.h 2017-06-16 11:06:53.329712682 -0400
+++ new/indent_globs.h 2017-06-17 14:45:41.388015754 -0400
@@ -43,8 +43,12 @@
* of code */


+#ifndef false
#define false 0
+#endif
+#ifndef true
#define true 1
+#endif


FILE *input; /* the fid for the input file */

Other than that, I think this is a workable compromise on the
portability questions.

regards, tom lane

Attachment Content-Type Size
err.h text/plain 2.0 KB
err.c text/plain 2.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2017-06-17 19:55:48 Re: INSERT ... ON CONFLICT () SELECT
Previous Message Thomas Munro 2017-06-17 19:50:55 Re: Decimal64 and Decimal128