From 664a6298ee2b5529122383c67bf85501abdbd21e Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 28 Mar 2018 20:45:44 +0300 Subject: [PATCH] Make handling of variable-length array more idiomatic. While we're at it, remove the mock tests on SessionState_ShmemSize. IMHO it was always a bit silly, it almost literally duplicated the computation in SessionState_ShmemSize() function itself. And now even more so. --- src/backend/utils/session_state.c | 10 +-- src/backend/utils/test/session_state_test.c | 89 --------------------- src/include/utils/session_state.h | 7 +- 3 files changed, 7 insertions(+), 99 deletions(-) diff --git a/src/backend/utils/session_state.c b/src/backend/utils/session_state.c index 083da047e2..ac98d0ca88 100644 --- a/src/backend/utils/session_state.c +++ b/src/backend/utils/session_state.c @@ -223,7 +223,7 @@ SessionState_ShmemSize() { SessionStateArrayEntryCount = MaxBackends; - Size size = offsetof(SessionStateArray, data); + Size size = offsetof(SessionStateArray, sessions); size = add_size(size, mul_size(sizeof(SessionState), SessionStateArrayEntryCount)); @@ -252,23 +252,21 @@ SessionState_ShmemInit() AllSessionStateEntries->numSession = 0; AllSessionStateEntries->maxSession = SessionStateArrayEntryCount; - AllSessionStateEntries->sessions = (SessionState *)&AllSessionStateEntries->data; - /* Every entry of the array is free at this time */ - AllSessionStateEntries->freeList = AllSessionStateEntries->sessions; + AllSessionStateEntries->freeList = (SessionState *) &AllSessionStateEntries->sessions[0]; AllSessionStateEntries->usedList = NULL; /* * Set all the entries' sessionId to invalid. Also, set the next pointer * to point to the next entry in the array. */ - SessionState *prev = &AllSessionStateEntries->sessions[0]; + SessionState *prev = (SessionState *) &AllSessionStateEntries->sessions[0]; prev->sessionId = INVALID_SESSION_ID; prev->cleanupCountdown = CLEANUP_COUNTDOWN_BEFORE_RUNAWAY; for (int i = 1; i < AllSessionStateEntries->maxSession; i++) { - SessionState *cur = &AllSessionStateEntries->sessions[i]; + SessionState *cur = (SessionState *) &AllSessionStateEntries->sessions[i]; cur->sessionId = INVALID_SESSION_ID; cur->cleanupCountdown = CLEANUP_COUNTDOWN_BEFORE_RUNAWAY; diff --git a/src/backend/utils/test/session_state_test.c b/src/backend/utils/test/session_state_test.c index c6a72d158d..eee98b4c56 100755 --- a/src/backend/utils/test/session_state_test.c +++ b/src/backend/utils/test/session_state_test.c @@ -5,9 +5,6 @@ #include "postgres.h" -static Size add_size(Size s1, Size s2); -static Size mul_size(Size s1, Size s2); - #include "../session_state.c" /* @@ -60,50 +57,6 @@ _ExceptionalCondition() PG_RE_THROW(); } -static Size -GetSessionStateArrayHeaderSize() -{ - return sizeof(int) /* numSession */ + - sizeof(int) /* maxSession*/ + sizeof(SessionState *) /* freeList */ + - sizeof(SessionState *) /* usedList */+ sizeof(SessionState *) /* sessions */; -} - -/* - * Add two Size values, checking for overflow - */ -static Size -add_size(Size s1, Size s2) -{ - Size result; - - result = s1 + s2; - /* We are assuming Size is an unsigned type here... */ - if (result < s1 || result < s2) - ereport(ERROR, - (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("requested shared memory size overflows size_t"))); - return result; -} - -/* - * Multiply two Size values, checking for overflow - */ -static Size -mul_size(Size s1, Size s2) -{ - Size result; - - if (s1 == 0 || s2 == 0) - return 0; - result = s1 * s2; - /* We are assuming Size is an unsigned type here... */ - if (result / s2 != s1) - ereport(ERROR, - (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("requested shared memory size overflows size_t"))); - return result; -} - /* Creates a SessionStateArray of the specified number of entry */ static void CreateSessionStateArray(int numEntries) @@ -208,43 +161,6 @@ ReleaseSessionState(int sessionId) SessionState_Shutdown(); } -/* - * Checks if the SessionStateArray struct layout is as expected - */ -void -test__SessionState_ShmemSize__StructLayout(void **state) -{ - const int headerEndOffset = offsetof(SessionStateArray, data); - /* - * Make sure the data field is the last field. - */ - assert_true(headerEndOffset == sizeof(SessionStateArray) - sizeof(void*)); - - Size actualSize = sizeof(SessionStateArray); - Size calculatedSize = GetSessionStateArrayHeaderSize() + - sizeof(void *) /* the start pointer of the entries */; - assert_true(actualSize == calculatedSize); -} - -/* - * Checks if the SessionState_ShmemSize calculates correct size - */ -void -test__SessionState_ShmemSize__CalculatesCorrectSize(void **state) -{ - const Size headerSize = GetSessionStateArrayHeaderSize(); - - MaxBackends = 0; - assert_true(headerSize == SessionState_ShmemSize()); - - MaxBackends = 10; - assert_true(headerSize + 10 * sizeof(SessionState) == SessionState_ShmemSize()); - - /* Current maximum value for Maxbackends is INT_MAX / BLCKSZ */ - MaxBackends = MAX_MAX_BACKENDS; - assert_true(headerSize + (MAX_MAX_BACKENDS) * sizeof(SessionState) == SessionState_ShmemSize()); -} - /* * Checks if SessionState_ShmemInit does nothing under postmaster. * Note, it is *only* expected to re-attach with an existing array. @@ -259,7 +175,6 @@ test__SessionState_ShmemInit__NoOpUnderPostmaster(void **state) /* Initilize with some non-zero values */ fakeSessionStateArray.maxSession = 0; fakeSessionStateArray.numSession = 0; - fakeSessionStateArray.sessions = NULL; fakeSessionStateArray.freeList = NULL; fakeSessionStateArray.usedList = NULL; @@ -277,7 +192,6 @@ test__SessionState_ShmemInit__NoOpUnderPostmaster(void **state) /* All the struct properties should be unchanged */ assert_true(AllSessionStateEntries->maxSession == 0); assert_true(AllSessionStateEntries->numSession == 0); - assert_true(AllSessionStateEntries->sessions == NULL); assert_true(AllSessionStateEntries->freeList == NULL && AllSessionStateEntries->usedList == NULL); @@ -303,7 +217,6 @@ test__SessionState_ShmemInit__InitializesWhenPostmaster(void **state) /* All the struct properties should be unchanged */ assert_true(AllSessionStateEntries->maxSession == MaxBackends); assert_true(AllSessionStateEntries->numSession == 0); - assert_true(AllSessionStateEntries->sessions == &AllSessionStateEntries->data); assert_true(AllSessionStateEntries->freeList == AllSessionStateEntries->sessions && AllSessionStateEntries->usedList == NULL); @@ -603,8 +516,6 @@ main(int argc, char* argv[]) gp_sessionstate_loglevel = LOG; const UnitTest tests[] = { - unit_test(test__SessionState_ShmemSize__StructLayout), - unit_test(test__SessionState_ShmemSize__CalculatesCorrectSize), unit_test(test__SessionState_ShmemInit__NoOpUnderPostmaster), unit_test(test__SessionState_ShmemInit__InitializesWhenPostmaster), unit_test(test__SessionState_ShmemInit__LinkedListSanity), diff --git a/src/include/utils/session_state.h b/src/include/utils/session_state.h index 0e669523fe..af57690a9c 100644 --- a/src/include/utils/session_state.h +++ b/src/include/utils/session_state.h @@ -115,10 +115,9 @@ typedef struct SessionStateArray SessionState *freeList; /* Head of the list of used entries */ SessionState *usedList; - /* Pointer to the head of the entries */ - SessionState *sessions; - /* Placeholder to find the address where the array of entries begin */ - void *data; + + /* Head of the entries */ + SessionState sessions[1]; /* VARIABLE LENGTH ARRAY */ } SessionStateArray; extern volatile SessionState *MySessionState; -- GitLab