pgsql: dblink: Replace some macros by static functions

On 11 March 2017 at 03:46, Peter Eisentraut <peter_e@gmx.net> wrote:

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/acaf7ccb94a3916ea83712671a3563
f0eb595558

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

Hi Peter,

Please accept a small patch which fixes a new compiler warning which
started as a result of this commit.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers