From 5e63a5f8e467c7855ab22fa7e5fbdac94d9c87e7 Mon Sep 17 00:00:00 2001 From: Craig Ringer Date: Fri, 1 Apr 2016 13:36:31 +0800 Subject: [PATCH] Fix test_slot_timelines buildfarm failure on 32-bit ARM The code was reading a NULL 64-bit Datum that is pass-by-value on 64-bit platforms but pass-by-reference on 32-bit. On 64-bit this just produces zero, but on 32-bit it'll dereference a NULL pointer. Since InvalidXLogRecPtr is zero this went unnoticed. Test for NULL arguments. In the process, switch to taking 'xid'-typed arguments instead of bigint. Since txid_current() returns bigint and there's no cast from bigint to xid this requires a cast through 'text' in our tests. --- .../expected/load_extension.out | 2 +- .../test_slot_timelines/sql/load_extension.sql | 2 +- .../test_slot_timelines--1.0.sql | 4 +-- .../test_slot_timelines/test_slot_timelines.c | 31 +++++++++++++++++++--- 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/test/modules/test_slot_timelines/expected/load_extension.out b/src/test/modules/test_slot_timelines/expected/load_extension.out index 14a414a..7c2ad9d 100644 --- a/src/test/modules/test_slot_timelines/expected/load_extension.out +++ b/src/test/modules/test_slot_timelines/expected/load_extension.out @@ -5,7 +5,7 @@ SELECT test_slot_timelines_create_logical_slot('test_slot', 'test_decoding'); (1 row) -SELECT test_slot_timelines_advance_logical_slot('test_slot', txid_current(), txid_current(), pg_current_xlog_location(), pg_current_xlog_location()); +SELECT test_slot_timelines_advance_logical_slot('test_slot', txid_current()::text::xid, txid_current()::text::xid, pg_current_xlog_location(), pg_current_xlog_location()); test_slot_timelines_advance_logical_slot ------------------------------------------ diff --git a/src/test/modules/test_slot_timelines/sql/load_extension.sql b/src/test/modules/test_slot_timelines/sql/load_extension.sql index a71127d..2440355 100644 --- a/src/test/modules/test_slot_timelines/sql/load_extension.sql +++ b/src/test/modules/test_slot_timelines/sql/load_extension.sql @@ -2,6 +2,6 @@ CREATE EXTENSION test_slot_timelines; SELECT test_slot_timelines_create_logical_slot('test_slot', 'test_decoding'); -SELECT test_slot_timelines_advance_logical_slot('test_slot', txid_current(), txid_current(), pg_current_xlog_location(), pg_current_xlog_location()); +SELECT test_slot_timelines_advance_logical_slot('test_slot', txid_current()::text::xid, txid_current()::text::xid, pg_current_xlog_location(), pg_current_xlog_location()); SELECT pg_drop_replication_slot('test_slot'); diff --git a/src/test/modules/test_slot_timelines/test_slot_timelines--1.0.sql b/src/test/modules/test_slot_timelines/test_slot_timelines--1.0.sql index 31d7f8e..50449bf 100644 --- a/src/test/modules/test_slot_timelines/test_slot_timelines--1.0.sql +++ b/src/test/modules/test_slot_timelines/test_slot_timelines--1.0.sql @@ -8,9 +8,9 @@ LANGUAGE c AS 'MODULE_PATHNAME'; COMMENT ON FUNCTION test_slot_timelines_create_logical_slot(text, text) IS 'Create a logical slot at a particular lsn and xid. Do not use in production servers, it is not safe. The slot is created with an invalid xmin and lsn.'; -CREATE OR REPLACE FUNCTION test_slot_timelines_advance_logical_slot(slot_name text, new_xmin bigint, new_catalog_xmin bigint, new_restart_lsn pg_lsn, new_confirmed_lsn pg_lsn) +CREATE OR REPLACE FUNCTION test_slot_timelines_advance_logical_slot(slot_name text, new_xmin xid, new_catalog_xmin xid, new_restart_lsn pg_lsn, new_confirmed_lsn pg_lsn) RETURNS void LANGUAGE c AS 'MODULE_PATHNAME'; -COMMENT ON FUNCTION test_slot_timelines_advance_logical_slot(text, bigint, bigint, pg_lsn, pg_lsn) +COMMENT ON FUNCTION test_slot_timelines_advance_logical_slot(text, xid, xid, pg_lsn, pg_lsn) IS 'Advance a logical slot directly. Do not use this in production servers, it is not safe.'; diff --git a/src/test/modules/test_slot_timelines/test_slot_timelines.c b/src/test/modules/test_slot_timelines/test_slot_timelines.c index 74dd1a0..e818705 100644 --- a/src/test/modules/test_slot_timelines/test_slot_timelines.c +++ b/src/test/modules/test_slot_timelines/test_slot_timelines.c @@ -85,10 +85,33 @@ Datum test_slot_timelines_advance_logical_slot(PG_FUNCTION_ARGS) { char *slotname = text_to_cstring(PG_GETARG_TEXT_P(0)); - TransactionId new_xmin = (TransactionId) PG_GETARG_INT64(1); - TransactionId new_catalog_xmin = (TransactionId) PG_GETARG_INT64(2); - XLogRecPtr restart_lsn = PG_GETARG_LSN(3); - XLogRecPtr confirmed_lsn = PG_GETARG_LSN(4); + TransactionId new_xmin; + TransactionId new_catalog_xmin; + XLogRecPtr restart_lsn; + XLogRecPtr confirmed_lsn; + + if (PG_ARGISNULL(0)) + elog(ERROR, "slot name may not be null"); + + if (PG_ARGISNULL(1)) + new_xmin = InvalidTransactionId; + else + new_xmin = (TransactionId) DatumGetTransactionId(PG_GETARG_DATUM(1)); + + if (PG_ARGISNULL(2)) + new_catalog_xmin = InvalidTransactionId; + else + new_catalog_xmin = (TransactionId) DatumGetTransactionId(PG_GETARG_DATUM(2)); + + if (PG_ARGISNULL(3)) + restart_lsn = InvalidXLogRecPtr; + else + restart_lsn = PG_GETARG_LSN(3); + + if (PG_ARGISNULL(4)) + confirmed_lsn = InvalidXLogRecPtr; + else + confirmed_lsn = PG_GETARG_LSN(4); CheckSlotRequirements(); -- 2.1.0