Thread: [COMMITTERS] pgsql: dblink: Replace some macros by static functions

dblink: Replace some macros by static functions

Also remove some unused code and the no longer useful dblink.h file.

Reviewed-by: Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/acaf7ccb94a3916ea83712671a3563f0eb595558

Modified Files
--------------
contrib/dblink/dblink.c | 290 +++++++++++++++++++++++-------------------------
contrib/dblink/dblink.h |  39 -------
2 files changed, 137 insertions(+), 192 deletions(-)

On 3/12/17 17:44, David Rowley wrote:
> Please accept a small patch which fixes a new compiler warning which
> started as a result of this commit.

Done.

Which compiler is that?


--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 3/12/17 17:44, David Rowley wrote:
>> Please accept a small patch which fixes a new compiler warning which
>> started as a result of this commit.

> Done.

> Which compiler is that?

Any compiler that didn't support pg_attribute_noreturn() could be expected
to whine about the original coding.

In the same vein, I think this bit in dblink_open is pretty poor coding
practice:

    if (!rconn || !rconn->conn)
        dblink_conn_not_avail(conname);
    else
        conn = rconn->conn;

as it expects both the compiler and the reader to understand that
we will not proceed without "conn" getting a value.  I see that
that was band-aided around by initializing conn to NULL, but that's a
crummy way of suppressing uninitialized-variable warnings, because it
will mask even actual errors.  Far better would be to remove the dummy
initialization and write

    if (!rconn || !rconn->conn)
        dblink_conn_not_avail(conname);
    conn = rconn->conn;


            regards, tom lane

On 3/13/17 15:58, Tom Lane wrote:
> In the same vein, I think this bit in dblink_open is pretty poor coding
> practice:
>
>     if (!rconn || !rconn->conn)
>         dblink_conn_not_avail(conname);
>     else
>         conn = rconn->conn;
>
> as it expects both the compiler and the reader to understand that
> we will not proceed without "conn" getting a value.  I see that
> that was band-aided around by initializing conn to NULL, but that's a
> crummy way of suppressing uninitialized-variable warnings, because it
> will mask even actual errors.  Far better would be to remove the dummy
> initialization and write
>
>     if (!rconn || !rconn->conn)
>         dblink_conn_not_avail(conname);
>     conn = rconn->conn;

fixed

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services