Horton

Re: Ordering of header file inclusion

vignesh C <vignesh21@gmail.com>

View thread
From: vignesh C <vignesh21@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Kuntal Ghosh <kuntalghosh.2007@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, Andres Freund <andres@anarazel.de>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2019-11-22T04:14:20Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Make the order of the header file includes consistent.

  2. Make the order of the header file includes consistent in backend modules.

  3. Make the order of the header file includes consistent in non-backend modules.

  4. Make the order of the header file includes consistent in contrib modules.

On Thu, Nov 21, 2019 at 2:11 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Sat, Nov 16, 2019 at 7:01 AM vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Tue, Nov 12, 2019 at 11:19 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Tue, Nov 12, 2019 at 6:33 AM vignesh C <vignesh21@gmail.com> wrote:
> > > >
> > > >
> > > > Thanks Amit for your comments. Please find the updated patch which
> > > > does not include the changes mentioned above.
> > > >
> > >
> > > Thanks for working on this.  I have pushed your latest patch.
> > >
> >
> > Thanks Amit for pushing the patch. I have re-verified and found that
> > changes need  to be done in few more places. The main changes are made
> > in the header file and plpython source files. The attached patch
> > handles the same. I have verified make check and make check-world
> > including --with-python & --with-perl in the following:
> > CentOS Linux release 7.7.1908
> > Red Hat Enterprise Linux Server release 7.1
> >
> > I have verified including --llvm in CentOS Linux release 7.7.1908.
> >
>
> Thanks for finding the remaining places, the patch looks good to me.
> I hope this covers the entire code.  BTW, are you using some script to
> find this or is this a result of manual inspection of code?  I have
> modified the commit message in the attached patch.  I will commit this
> early next week unless someone else wants to review it.
>

I have used script to verify if the inclusions are sorted. There are
few files which I did not modify intentionally, they are mainly like
the below type as in uuid-ossp.c:
#include "postgres.h"

#include "fmgr.h"
#include "port/pg_bswap.h"
#include "utils/builtins.h"
#include "utils/uuid.h"

/*
 * It's possible that there's more than one uuid.h header file present.
 * We expect configure to set the HAVE_ symbol for only the one we want.
 *
 * BSD includes a uuid_hash() function that conflicts with the one in
 * builtins.h; we #define it out of the way.
 */
#define uuid_hash bsd_uuid_hash

#if defined(HAVE_UUID_H)
#include <uuid.h>
#elif defined(HAVE_OSSP_UUID_H)
#include <ossp/uuid.h>
#elif defined(HAVE_UUID_UUID_H)
#include <uuid/uuid.h>


After the inclusion they have define and further include based on #if
defined. In few cases I had seen the include happens at the end of the
file like in regcomp.c as there may be impact. I felt it is better not
to change these files. Let me know your thoughts on the same.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com