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: 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-10-22T17:22:15Z
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.

Attachments

On Tue, Oct 22, 2019 at 12:56 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> Few comments on 0001-Ordering-of-header-files-in-contrib-dir-oct21.patch
> -----------------------------------------------------------------------------------------------------------
> 1.
> --- a/contrib/isn/isn.c
> +++ b/contrib/isn/isn.c
> @@ -15,9 +15,9 @@
>  #include "postgres.h"
>
>  #include "fmgr.h"
> +#include "isn.h"
>  #include "utils/builtins.h"
>
> -#include "isn.h"
>  #include "EAN13.h"
>  #include "ISBN.h"
>  #include "ISMN.h"
>
> Why only "isn.h" is moved and not others?
>
Fixed.
Order is based on ascii table. Upper case letters will come before
lower case letters.
> 2.
> +++ b/contrib/pgcrypto/px-crypt.c
> @@ -31,9 +31,8 @@
>
>  #include "postgres.h"
>
> -#include "px.h"
>  #include "px-crypt.h"
> -
> +#include "px.h"
>
> I think such ordering was fine.  Forex. see, hash.c (hash.h was
> included first and then hash_xlog.h).
>
Order is based on ascii table.
Ascii value for "." is 46, Ascii value for "-" is 45.
Hence have placed like:
#include "px-crypt.h"
#include "px.h"
Not make any changes for this. If still required I can modify.
> 3.
> +#include "_int.h"
>  #include "access/gist.h"
>  #include "access/stratnum.h"
>
> -#include "_int.h"
> -
>
> Do we need to give preference to '_'?  Is it being done somewhere
> else?  It is not that this is wrong, just that I am not sure about
> this.
>
The changes are done based on ascii table.
Ascii value of "_" is 95.
Ascii value of "a" is 97.
Hence _int.h is place before access/gist.h.
I have not made any changes for this. If still required I can modify.
> 4.
> --- a/contrib/hstore/hstore_io.c
> +++ b/contrib/hstore/hstore_io.c
> @@ -8,6 +8,7 @@
>  #include "access/htup_details.h"
>  #include "catalog/pg_type.h"
>  #include "funcapi.h"
> +#include "hstore.h"
>  #include "lib/stringinfo.h"
>  #include "libpq/pqformat.h"
>  #include "utils/builtins.h"
> @@ -18,7 +19,6 @@
>  #include "utils/memutils.h"
>  #include "utils/typcache.h"
>
> -#include "hstore.h"
>
>  PG_MODULE_MAGIC;
>
> This created an extra white line.
>
Fixed.
> 5.
> While reviewing, I noticed that in contrib/intarray/_int_op.c, there
> is an extra white line between postgres.h and its first include.  I
> think we can make that as well consistent.
>
Fixed.
Thanks for the comments.
Attached patch has the updated changes.


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