Hackers,

Please find access/xlog_internal.h refactored in the attached patch series.  
This header is included from many places, including external tools.  It is 
aesthetically displeasing to have something called "internal" used from so many 
places, especially when many of those places do not deal directly with the 
internal workings of xlog.  But it is even worse that multiple files include 
this header for no reason.

A small portion of access/xlog_internal.h defines the RmgrData struct, and in 
support of this struct the header includes a number of other headers.  Files 
that include access/xlog_internal.h indirectly include these other headers, 
which most do not need. (Only 3 out of 41 files involved actually need that 
portion of the header.)  For third-party tools which deal with backup, restore, 
or replication matters, including xlog_internal.h is necessary to get macros 
for calculating xlog file names, but doing so also indirectly pulls in other 
headers, increasing the risk of unwanted symbol collisions.  Some colleagues 
and I ran into this exact problem in a C++ program that uses both 
xlog_internal.h and the Boost C++ library.


0001 - Removes gratuitous inclusion of access/xlog_internal.h from *.c files in 
core that are currently including it without need.  The following files are so 
modified:

    src/backend/access/transam/rmgr.c
    src/backend/postmaster/bgwriter.c
    src/backend/replication/logical/decode.c
    src/backend/replication/logical/logical.c
    src/backend/replication/logical/logicalfuncs.c
    src/backend/replication/logical/worker.c
    src/bin/pg_basebackup/pg_recvlogical.c
    src/bin/pg_rewind/timeline.c
    src/bin/pg_waldump/rmgrdesc.c

0002 - Moves RmgrData from access/xlog_internal.h into a new file 
access/rmgr_internal.h.  I clearly did not waste time thinking of a clever file 
name.  Bikeshedding welcome.  Most files which currently include 
xlog_internal.h do not need the definition of RmgrData.  As it stands now, 
inclusion of xlog_internal.h indirectly includes the following headers:

    <fcntl.h>
    "access/rmgr.h"
    "access/rmgrlist.h"
    "access/transam.h"
    "access/xlogdefs.h"
    "access/xlogreader.h"
    "access/xlogrecord.h"
    "catalog/catversion.h"
    "common/relpath.h"
    "datatype/timestamp.h"
    "lib/stringinfo.h"
    "pgtime.h"
    "port/pg_bswap.h"
    "port/pg_crc32c.h"
    "storage/backendid.h"
    "storage/block.h"
    "storage/relfilenode.h"

After refactoring, the inclusion of xlog_internal.h includes indirectly only 
these headers:

    <fcntl.h>
    "access/xlogdefs.h"
    "datatype/timestamp.h"
    "pgtime.h"

and only these files need to be altered to include the new rmgr_internal.h 
header:

    src/backend/access/transam/rmgr.c
    src/backend/access/transam/xlog.c
    src/backend/utils/misc/guc.c

Thoughts?

Attachment: v1-0001-Removing-gratuitous-inclusion-of-xlog_internal.h.patch
Description: Binary data

Attachment: v1-0002-Moving-RmgrData-from-xlog_internal.h-to-its-own-h.patch
Description: Binary data


—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Reply via email to