On 2017-05-18 18:13, Tom Lane wrote:
> Piotr Stefaniak <postgres@piotr-stefaniak.me> writes:
>> That, I assume, would be me. Coincidentally, I'm about to push my fixes
>> upstream (FreeBSD). Before that happens, my changes can be obtained from
>> https://github.com/pstef/freebsd_indent and tested, if anyone wishes.
>
> I spent a little bit of time on portability testing, because we are
> certainly going to insist that this tool be portable to more than
> just FreeBSD. Things are not very good as it stands:
>
> * Makefile is 100% BSD-specific. Possibly we could just agree to
> disagree on that point, and include a PG-style makefile that is not
> like upstream's. I attach the one I used for test purposes.
This would have to live outside of FreeBSD for obvious (or not) reasons.
Most likely as a part of pg_bsd_indent. I use the original ("BSD")
Makefile on Linux, feeding it to bmake(1). But I don't expect bmake to
become a requirement for pg_bsd_indent.
> * __FBSDID() macros fail to compile anywhere else than FreeBSD.
> Couldn't you hide those inside #if 0, as you're already doing for
> the ancient sccsid strings?
The use of __FBSDID macro won't be going anywhere from FreeBSD indent,
I'm afraid. But I think it could be if-def'd out under systems other
than FreeBSD.
> * Please put the copyright[] string in indent.c inside #if 0 too,
> as that draws unreferenced-variable warnings on some compilers.
>
> * There's one use of bcopy(), please replace with memmove().
These could probably be done upstream. I'd like to convert the strings
to comments.
> * References to <sys/cdefs.h> and <err.h> are problematic, as both
> are BSD-isms not found in POSIX. They seem to be available anyway
> on Linux, but I bet not on Windows or SysV-tradition boxes.
> I removed the <sys/cdefs.h> includes and didn't see any problems,
> but removing <err.h> exposes calls to err() and errx(), which
> we'd have to find replacements for. Maybe just change them to
> standard-conforming printf() + exit()?
I'll look into why indent includes sys/cdefs.h. I don't expect to be
allowed to replace err() and errx() with equivalent solutions based on
ISO C and POSIX. I fear the idea of detecting err*() support prior to
compilation... Probably a much simpler solution would be to replace
err*() with equivalent solutions in the PG's fork of indent. printf()
and exit() would probably be insufficient, you'd also need strerror(), I
think.
