From de1eb3c459d55144e1cb528781c87cafad5e5ff7 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Wed, 8 Nov 2017 12:19:07 +0900 Subject: [PATCH] Ensure shared memory functions handle NULL parameters correctly --- expected/repmgr_extension.out | 54 +++++++++++++++++++++++++++++++++++ repmgr.c | 46 ++++++++++++++++++++++++----- sql/repmgr_extension.sql | 9 ++++++ 3 files changed, 102 insertions(+), 7 deletions(-) diff --git a/expected/repmgr_extension.out b/expected/repmgr_extension.out index 779a7270..27442b80 100644 --- a/expected/repmgr_extension.out +++ b/expected/repmgr_extension.out @@ -38,6 +38,12 @@ SELECT repmgr.am_bdr_failover_handler(-1); (1 row) +SELECT repmgr.am_bdr_failover_handler(NULL); + am_bdr_failover_handler +------------------------- + +(1 row) + SELECT repmgr.get_new_primary(); get_new_primary ----------------- @@ -56,18 +62,60 @@ SELECT repmgr.notify_follow_primary(-1); (1 row) +SELECT repmgr.notify_follow_primary(NULL); + notify_follow_primary +----------------------- + +(1 row) + SELECT repmgr.other_node_is_candidate(-1,-1); other_node_is_candidate ------------------------- (1 row) +SELECT repmgr.other_node_is_candidate(-1,NULL); + other_node_is_candidate +------------------------- + +(1 row) + +SELECT repmgr.other_node_is_candidate(NULL,-1); + other_node_is_candidate +------------------------- + +(1 row) + +SELECT repmgr.other_node_is_candidate(NULL,NULL); + other_node_is_candidate +------------------------- + +(1 row) + SELECT repmgr.request_vote(-1,-1); request_vote -------------- (1 row) +SELECT repmgr.request_vote(-1,NULL); + request_vote +-------------- + +(1 row) + +SELECT repmgr.request_vote(NULL,-1); + request_vote +-------------- + +(1 row) + +SELECT repmgr.request_vote(NULL,NULL); + request_vote +-------------- + +(1 row) + SELECT repmgr.reset_voting_status(); reset_voting_status --------------------- @@ -80,6 +128,12 @@ SELECT repmgr.set_local_node_id(-1); (1 row) +SELECT repmgr.set_local_node_id(NULL); + set_local_node_id +------------------- + +(1 row) + SELECT repmgr.set_voting_status_initiated(); set_voting_status_initiated ----------------------------- diff --git a/repmgr.c b/repmgr.c index 26b34c6b..11632de4 100644 --- a/repmgr.c +++ b/repmgr.c @@ -227,11 +227,16 @@ repmgr_shmem_startup(void) Datum set_local_node_id(PG_FUNCTION_ARGS) { - int local_node_id = PG_GETARG_INT32(0); + int local_node_id = UNKNOWN_NODE_ID; if (!shared_state) PG_RETURN_NULL(); + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + + local_node_id = PG_GETARG_INT32(0); + LWLockAcquire(shared_state->lock, LW_EXCLUSIVE); /* only set local_node_id once, as it should never change */ @@ -301,14 +306,20 @@ request_vote(PG_FUNCTION_ARGS) #endif /* node_id used for logging purposes */ - int requesting_node_id = PG_GETARG_INT32(0); - int current_electoral_term = PG_GETARG_INT32(1); + int requesting_node_id = UNKNOWN_NODE_ID; + int current_electoral_term = UNKNOWN_NODE_ID; int ret; if (!shared_state) PG_RETURN_NULL(); + if (PG_ARGISNULL(0) || PG_ARGISNULL(1)) + PG_RETURN_NULL(); + + requesting_node_id = PG_GETARG_INT32(0); + current_electoral_term = PG_GETARG_INT32(1); + LWLockAcquire(shared_state->lock, LW_SHARED); /* only do something if local_node_id is initialised */ @@ -453,12 +464,19 @@ Datum other_node_is_candidate(PG_FUNCTION_ARGS) { #ifndef BDR_ONLY - int requesting_node_id = PG_GETARG_INT32(0); - int electoral_term = PG_GETARG_INT32(1); + + int requesting_node_id = UNKNOWN_NODE_ID; + int electoral_term = UNKNOWN_NODE_ID; if (!shared_state) PG_RETURN_NULL(); + if (PG_ARGISNULL(0) || PG_ARGISNULL(1)) + PG_RETURN_NULL(); + + requesting_node_id = PG_GETARG_INT32(0); + electoral_term = PG_GETARG_INT32(1); + LWLockAcquire(shared_state->lock, LW_SHARED); /* only do something if local_node_id is initialised */ @@ -493,11 +511,16 @@ Datum notify_follow_primary(PG_FUNCTION_ARGS) { #ifndef BDR_ONLY - int primary_node_id = PG_GETARG_INT32(0); + int primary_node_id = UNKNOWN_NODE_ID; if (!shared_state) PG_RETURN_NULL(); + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + + primary_node_id = PG_GETARG_INT32(0); + LWLockAcquire(shared_state->lock, LW_SHARED); /* only do something if local_node_id is initialised */ @@ -536,6 +559,10 @@ get_new_primary(PG_FUNCTION_ARGS) LWLockRelease(shared_state->lock); #endif + + if (new_primary_node_id == UNKNOWN_NODE_ID) + PG_RETURN_NULL(); + PG_RETURN_INT32(new_primary_node_id); } @@ -569,12 +596,17 @@ reset_voting_status(PG_FUNCTION_ARGS) Datum am_bdr_failover_handler(PG_FUNCTION_ARGS) { - int node_id = PG_GETARG_INT32(0); + int node_id = UNKNOWN_NODE_ID; bool am_handler = false; if (!shared_state) PG_RETURN_NULL(); + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + + node_id = PG_GETARG_INT32(0); + LWLockAcquire(shared_state->lock, LW_SHARED); if (shared_state->bdr_failover_handler == UNKNOWN_NODE_ID) diff --git a/sql/repmgr_extension.sql b/sql/repmgr_extension.sql index d2d2a376..cafb2be6 100644 --- a/sql/repmgr_extension.sql +++ b/sql/repmgr_extension.sql @@ -18,13 +18,22 @@ SELECT * FROM repmgr.show_nodes; -- functions SELECT repmgr.am_bdr_failover_handler(-1); +SELECT repmgr.am_bdr_failover_handler(NULL); SELECT repmgr.get_new_primary(); SELECT repmgr.get_voting_status(); SELECT repmgr.notify_follow_primary(-1); +SELECT repmgr.notify_follow_primary(NULL); SELECT repmgr.other_node_is_candidate(-1,-1); +SELECT repmgr.other_node_is_candidate(-1,NULL); +SELECT repmgr.other_node_is_candidate(NULL,-1); +SELECT repmgr.other_node_is_candidate(NULL,NULL); SELECT repmgr.request_vote(-1,-1); +SELECT repmgr.request_vote(-1,NULL); +SELECT repmgr.request_vote(NULL,-1); +SELECT repmgr.request_vote(NULL,NULL); SELECT repmgr.reset_voting_status(); SELECT repmgr.set_local_node_id(-1); +SELECT repmgr.set_local_node_id(NULL); SELECT repmgr.set_voting_status_initiated(); SELECT repmgr.standby_get_last_updated(); SELECT repmgr.standby_set_last_updated();