From a2ed45e2066b36b5fb00a1aeb09fb07822d3308d Mon Sep 17 00:00:00 2001 From: Brian Cloutier Date: Tue, 24 Oct 2017 13:13:13 -0700 Subject: [PATCH] Remove variable length arrays VLAs aren't supported by Visual Studio. - Remove all existing instances of VLAs. - Add a flag, -Werror=vla, which makes gcc refuse to compile if we add VLAs in the future. --- configure | 49 ++++++++++++++++++- configure.in | 1 + .../distributed/connection/remote_commands.c | 11 +++-- .../master/master_metadata_utility.c | 4 +- .../distributed/metadata/metadata_sync.c | 5 +- .../test/distributed_deadlock_detection.c | 5 +- .../distributed/transaction/backend_data.c | 10 ++-- .../distributed/utils/colocation_utils.c | 6 +-- .../distributed/utils/metadata_cache.c | 2 +- src/backend/distributed/utils/node_metadata.c | 4 +- src/include/distributed/metadata_sync.h | 1 - src/include/distributed/remote_commands.h | 2 + 12 files changed, 75 insertions(+), 25 deletions(-) diff --git a/configure b/configure index b97e4ab32..7c7a839cb 100755 --- a/configure +++ b/configure @@ -661,6 +661,7 @@ infodir docdir oldincludedir includedir +runstatedir localstatedir sharedstatedir sysconfdir @@ -737,6 +738,7 @@ datadir='${datarootdir}' sysconfdir='${prefix}/etc' sharedstatedir='${prefix}/com' localstatedir='${prefix}/var' +runstatedir='${localstatedir}/run' includedir='${prefix}/include' oldincludedir='/usr/include' docdir='${datarootdir}/doc/${PACKAGE_TARNAME}' @@ -989,6 +991,15 @@ do | -silent | --silent | --silen | --sile | --sil) silent=yes ;; + -runstatedir | --runstatedir | --runstatedi | --runstated \ + | --runstate | --runstat | --runsta | --runst | --runs \ + | --run | --ru | --r) + ac_prev=runstatedir ;; + -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \ + | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \ + | --run=* | --ru=* | --r=*) + runstatedir=$ac_optarg ;; + -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb) ac_prev=sbindir ;; -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \ @@ -1126,7 +1137,7 @@ fi for ac_var in exec_prefix prefix bindir sbindir libexecdir datarootdir \ datadir sysconfdir sharedstatedir localstatedir includedir \ oldincludedir docdir infodir htmldir dvidir pdfdir psdir \ - libdir localedir mandir + libdir localedir mandir runstatedir do eval ac_val=\$$ac_var # Remove trailing slashes. @@ -1279,6 +1290,7 @@ Fine tuning of the installation directories: --sysconfdir=DIR read-only single-machine data [PREFIX/etc] --sharedstatedir=DIR modifiable architecture-independent data [PREFIX/com] --localstatedir=DIR modifiable single-machine data [PREFIX/var] + --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run] --libdir=DIR object code libraries [EPREFIX/lib] --includedir=DIR C header files [PREFIX/include] --oldincludedir=DIR C header files for non-gcc [/usr/include] @@ -4007,6 +4019,41 @@ if test x"$citusac_cv_prog_cc_cflags__Wmissing_prototypes" = x"yes"; then CITUS_CFLAGS="$CITUS_CFLAGS -Wmissing-prototypes" fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CC supports -Werror=vla" >&5 +$as_echo_n "checking whether $CC supports -Werror=vla... " >&6; } +if ${citusac_cv_prog_cc_cflags__Werror_vla+:} false; then : + $as_echo_n "(cached) " >&6 +else + citusac_save_CFLAGS=$CFLAGS +CFLAGS="$citusac_save_CFLAGS -Werror=vla" +ac_save_c_werror_flag=$ac_c_werror_flag +ac_c_werror_flag=yes +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO"; then : + citusac_cv_prog_cc_cflags__Werror_vla=yes +else + citusac_cv_prog_cc_cflags__Werror_vla=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +ac_c_werror_flag=$ac_save_c_werror_flag +CFLAGS="$citusac_save_CFLAGS" +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $citusac_cv_prog_cc_cflags__Werror_vla" >&5 +$as_echo "$citusac_cv_prog_cc_cflags__Werror_vla" >&6; } +if test x"$citusac_cv_prog_cc_cflags__Werror_vla" = x"yes"; then + CITUS_CFLAGS="$CITUS_CFLAGS -Werror=vla" +fi + # visual studio does not support these # # --enable-coverage enables generation of code coverage metrics with gcov diff --git a/configure.in b/configure.in index e4fc454f7..75e4e0c88 100644 --- a/configure.in +++ b/configure.in @@ -163,6 +163,7 @@ CITUSAC_PROG_CC_CFLAGS_OPT([-Wendif-labels]) CITUSAC_PROG_CC_CFLAGS_OPT([-Wmissing-format-attribute]) CITUSAC_PROG_CC_CFLAGS_OPT([-Wmissing-declarations]) CITUSAC_PROG_CC_CFLAGS_OPT([-Wmissing-prototypes]) +CITUSAC_PROG_CC_CFLAGS_OPT([-Werror=vla]) # visual studio does not support these # # --enable-coverage enables generation of code coverage metrics with gcov diff --git a/src/backend/distributed/connection/remote_commands.c b/src/backend/distributed/connection/remote_commands.c index 00bd0de81..35c78b419 100644 --- a/src/backend/distributed/connection/remote_commands.c +++ b/src/backend/distributed/connection/remote_commands.c @@ -744,11 +744,16 @@ WaitForAllConnections(List *connectionList, bool raiseInterrupts) int connectionIndex = 0; ListCell *connectionCell = NULL; - MultiConnection *allConnections[totalConnectionCount]; - WaitEvent events[totalConnectionCount]; - bool connectionReady[totalConnectionCount]; + MultiConnection *allConnections[REMOTE_MAX_CONNECTIONS]; + WaitEvent events[REMOTE_MAX_CONNECTIONS]; + bool connectionReady[REMOTE_MAX_CONNECTIONS]; WaitEventSet *waitEventSet = NULL; + if (totalConnectionCount > REMOTE_MAX_CONNECTIONS) + { + ereport(ERROR, (errmsg("too many connections"))); + } + /* convert connection list to an array such that we can move items around */ foreach(connectionCell, connectionList) { diff --git a/src/backend/distributed/master/master_metadata_utility.c b/src/backend/distributed/master/master_metadata_utility.c index 19b1e5c52..0bbb849b7 100644 --- a/src/backend/distributed/master/master_metadata_utility.c +++ b/src/backend/distributed/master/master_metadata_utility.c @@ -637,7 +637,7 @@ NodeGroupHasShardPlacements(uint32 groupId, bool onlyConsiderActivePlacements) HeapTuple heapTuple = NULL; SysScanDesc scanDescriptor = NULL; - ScanKeyData scanKey[scanKeyCount]; + ScanKeyData scanKey[2]; Relation pgPlacement = heap_open(DistPlacementRelationId(), AccessShareLock); @@ -1097,7 +1097,7 @@ DeleteShardPlacementRow(uint64 placementId) Relation pgDistPlacement = NULL; SysScanDesc scanDescriptor = NULL; const int scanKeyCount = 1; - ScanKeyData scanKey[scanKeyCount]; + ScanKeyData scanKey[1]; bool indexOK = true; HeapTuple heapTuple = NULL; TupleDesc tupleDescriptor = NULL; diff --git a/src/backend/distributed/metadata/metadata_sync.c b/src/backend/distributed/metadata/metadata_sync.c index d7ff73334..0cc076b48 100644 --- a/src/backend/distributed/metadata/metadata_sync.c +++ b/src/backend/distributed/metadata/metadata_sync.c @@ -813,11 +813,10 @@ static void MarkNodeHasMetadata(char *nodeName, int32 nodePort, bool hasMetadata) { const bool indexOK = false; - const int scanKeyCount = 2; Relation pgDistNode = NULL; TupleDesc tupleDescriptor = NULL; - ScanKeyData scanKey[scanKeyCount]; + ScanKeyData scanKey[2]; SysScanDesc scanDescriptor = NULL; HeapTuple heapTuple = NULL; Datum values[Natts_pg_dist_node]; @@ -833,7 +832,7 @@ MarkNodeHasMetadata(char *nodeName, int32 nodePort, bool hasMetadata) BTEqualStrategyNumber, F_INT4EQ, Int32GetDatum(nodePort)); scanDescriptor = systable_beginscan(pgDistNode, InvalidOid, indexOK, - NULL, scanKeyCount, scanKey); + NULL, 2, scanKey); heapTuple = systable_getnext(scanDescriptor); if (!HeapTupleIsValid(heapTuple)) diff --git a/src/backend/distributed/test/distributed_deadlock_detection.c b/src/backend/distributed/test/distributed_deadlock_detection.c index 6f10f55fc..22f73c6a7 100644 --- a/src/backend/distributed/test/distributed_deadlock_detection.c +++ b/src/backend/distributed/test/distributed_deadlock_detection.c @@ -49,9 +49,8 @@ get_adjacency_list_wait_graph(PG_FUNCTION_ARGS) HASH_SEQ_STATUS status; TransactionNode *transactionNode = NULL; - const int attributeCount = 2; - Datum values[attributeCount]; - bool isNulls[attributeCount]; + Datum values[2]; + bool isNulls[2]; CheckCitusVersion(ERROR); diff --git a/src/backend/distributed/transaction/backend_data.c b/src/backend/distributed/transaction/backend_data.c index 2f1818de8..9be74f109 100644 --- a/src/backend/distributed/transaction/backend_data.c +++ b/src/backend/distributed/transaction/backend_data.c @@ -133,9 +133,8 @@ get_current_transaction_id(PG_FUNCTION_ARGS) TupleDesc tupleDescriptor = NULL; HeapTuple heapTuple = NULL; - const int attributeCount = 5; - Datum values[attributeCount]; - bool isNulls[attributeCount]; + Datum values[5]; + bool isNulls[5]; DistributedTransactionId *distributedTransctionId = NULL; @@ -196,9 +195,8 @@ get_all_active_transactions(PG_FUNCTION_ARGS) int backendIndex = 0; - const int attributeCount = 5; - Datum values[attributeCount]; - bool isNulls[attributeCount]; + Datum values[5]; + bool isNulls[5]; CheckCitusVersion(ERROR); diff --git a/src/backend/distributed/utils/colocation_utils.c b/src/backend/distributed/utils/colocation_utils.c index a5490714d..eb3573672 100644 --- a/src/backend/distributed/utils/colocation_utils.c +++ b/src/backend/distributed/utils/colocation_utils.c @@ -457,7 +457,7 @@ ColocationId(int shardCount, int replicationFactor, Oid distributionColumnType) HeapTuple colocationTuple = NULL; SysScanDesc scanDescriptor; const int scanKeyCount = 3; - ScanKeyData scanKey[scanKeyCount]; + ScanKeyData scanKey[3]; bool indexOK = true; Relation pgDistColocation = heap_open(DistColocationRelationId(), AccessShareLock); @@ -660,7 +660,7 @@ UpdateRelationColocationGroup(Oid distributedRelationId, uint32 colocationId) bool shouldSyncMetadata = false; bool indexOK = true; int scanKeyCount = 1; - ScanKeyData scanKey[scanKeyCount]; + ScanKeyData scanKey[1]; Datum values[Natts_pg_dist_partition]; bool isNull[Natts_pg_dist_partition]; bool replace[Natts_pg_dist_partition]; @@ -1052,7 +1052,7 @@ DeleteColocationGroup(uint32 colocationId) Relation pgDistColocation = NULL; SysScanDesc scanDescriptor = NULL; int scanKeyCount = 1; - ScanKeyData scanKey[scanKeyCount]; + ScanKeyData scanKey[1]; bool indexOK = false; HeapTuple heapTuple = NULL; diff --git a/src/backend/distributed/utils/metadata_cache.c b/src/backend/distributed/utils/metadata_cache.c index 055d8ff1d..bc0043a20 100644 --- a/src/backend/distributed/utils/metadata_cache.c +++ b/src/backend/distributed/utils/metadata_cache.c @@ -273,7 +273,7 @@ IsDistributedTableViaCatalog(Oid relationId) HeapTuple partitionTuple = NULL; SysScanDesc scanDescriptor = NULL; const int scanKeyCount = 1; - ScanKeyData scanKey[scanKeyCount]; + ScanKeyData scanKey[1]; bool indexOK = true; Relation pgDistPartition = heap_open(DistPartitionRelationId(), AccessShareLock); diff --git a/src/backend/distributed/utils/node_metadata.c b/src/backend/distributed/utils/node_metadata.c index 81ed80ddf..c2cb87824 100644 --- a/src/backend/distributed/utils/node_metadata.c +++ b/src/backend/distributed/utils/node_metadata.c @@ -1044,7 +1044,7 @@ GetNodeTuple(char *nodeName, int32 nodePort) const int scanKeyCount = 2; const bool indexOK = false; - ScanKeyData scanKey[scanKeyCount]; + ScanKeyData scanKey[2]; SysScanDesc scanDescriptor = NULL; HeapTuple heapTuple = NULL; HeapTuple nodeTuple = NULL; @@ -1289,7 +1289,7 @@ DeleteNodeRow(char *nodeName, int32 nodePort) HeapTuple heapTuple = NULL; SysScanDesc heapScan = NULL; - ScanKeyData scanKey[scanKeyCount]; + ScanKeyData scanKey[2]; Relation pgDistNode = heap_open(DistNodeRelationId(), RowExclusiveLock); diff --git a/src/include/distributed/metadata_sync.h b/src/include/distributed/metadata_sync.h index 6a4e6e517..aa5a7317f 100644 --- a/src/include/distributed/metadata_sync.h +++ b/src/include/distributed/metadata_sync.h @@ -56,5 +56,4 @@ extern void CreateTableMetadataOnWorkers(Oid relationId); "shardlength = EXCLUDED.shardlength, " \ "groupid = EXCLUDED.groupid" - #endif /* METADATA_SYNC_H */ diff --git a/src/include/distributed/remote_commands.h b/src/include/distributed/remote_commands.h index 9cab423fd..658706c49 100644 --- a/src/include/distributed/remote_commands.h +++ b/src/include/distributed/remote_commands.h @@ -17,6 +17,8 @@ #define QUERY_SEND_FAILED 1 #define RESPONSE_NOT_OKAY 2 +#define REMOTE_MAX_CONNECTIONS 1024 + struct pg_result; /* target of the PGresult typedef */ /* GUC, determining whether statements sent to remote nodes are logged */