From e553ca9e4ff484be281d01557615bdaee34a38a4 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 7 Oct 2024 23:49:22 +0300 Subject: [PATCH] Silence warnings about mixed declarations and code The warning: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] It's PostgreSQL project style to stick to the old C90 style. (Alternatively, we could disable it for our extension.) Part of the cleanup issue #9217. --- pgxn/neon/control_plane_connector.c | 201 +++++++++++++++------------- pgxn/neon/file_cache.c | 4 +- pgxn/neon/pagestore_smgr.c | 5 +- pgxn/neon/walproposer_pg.c | 7 +- pgxn/neon_walredo/walredoproc.c | 68 +++++----- 5 files changed, 153 insertions(+), 132 deletions(-) diff --git a/pgxn/neon/control_plane_connector.c b/pgxn/neon/control_plane_connector.c index de023da5c4..0730c305cb 100644 --- a/pgxn/neon/control_plane_connector.c +++ b/pgxn/neon/control_plane_connector.c @@ -146,6 +146,8 @@ ConstructDeltaMessage() if (RootTable.role_table) { JsonbValue roles; + HASH_SEQ_STATUS status; + RoleEntry *entry; roles.type = jbvString; roles.val.string.val = "roles"; @@ -153,9 +155,6 @@ ConstructDeltaMessage() pushJsonbValue(&state, WJB_KEY, &roles); pushJsonbValue(&state, WJB_BEGIN_ARRAY, NULL); - HASH_SEQ_STATUS status; - RoleEntry *entry; - hash_seq_init(&status, RootTable.role_table); while ((entry = hash_seq_search(&status)) != NULL) { @@ -190,10 +189,12 @@ ConstructDeltaMessage() } pushJsonbValue(&state, WJB_END_ARRAY, NULL); } - JsonbValue *result = pushJsonbValue(&state, WJB_END_OBJECT, NULL); - Jsonb *jsonb = JsonbValueToJsonb(result); + { + JsonbValue *result = pushJsonbValue(&state, WJB_END_OBJECT, NULL); + Jsonb *jsonb = JsonbValueToJsonb(result); - return JsonbToCString(NULL, &jsonb->root, 0 /* estimated_len */ ); + return JsonbToCString(NULL, &jsonb->root, 0 /* estimated_len */ ); + } } #define ERROR_SIZE 1024 @@ -272,32 +273,28 @@ SendDeltasToControlPlane() curl_easy_setopt(handle, CURLOPT_WRITEFUNCTION, ErrorWriteCallback); } - char *message = ConstructDeltaMessage(); - ErrorString str; - - str.size = 0; - - curl_easy_setopt(handle, CURLOPT_POSTFIELDS, message); - curl_easy_setopt(handle, CURLOPT_WRITEDATA, &str); - - const int num_retries = 5; - CURLcode curl_status; - - for (int i = 0; i < num_retries; i++) - { - if ((curl_status = curl_easy_perform(handle)) == 0) - break; - elog(LOG, "Curl request failed on attempt %d: %s", i, CurlErrorBuf); - pg_usleep(1000 * 1000); - } - if (curl_status != CURLE_OK) - { - elog(ERROR, "Failed to perform curl request: %s", CurlErrorBuf); - } - else { + char *message = ConstructDeltaMessage(); + ErrorString str; + const int num_retries = 5; + CURLcode curl_status; long response_code; + str.size = 0; + + curl_easy_setopt(handle, CURLOPT_POSTFIELDS, message); + curl_easy_setopt(handle, CURLOPT_WRITEDATA, &str); + + for (int i = 0; i < num_retries; i++) + { + if ((curl_status = curl_easy_perform(handle)) == 0) + break; + elog(LOG, "Curl request failed on attempt %d: %s", i, CurlErrorBuf); + pg_usleep(1000 * 1000); + } + if (curl_status != CURLE_OK) + elog(ERROR, "Failed to perform curl request: %s", CurlErrorBuf); + if (curl_easy_getinfo(handle, CURLINFO_RESPONSE_CODE, &response_code) != CURLE_UNKNOWN_OPTION) { if (response_code != 200) @@ -376,10 +373,11 @@ MergeTable() if (old_table->db_table) { - InitDbTableIfNeeded(); DbEntry *entry; HASH_SEQ_STATUS status; + InitDbTableIfNeeded(); + hash_seq_init(&status, old_table->db_table); while ((entry = hash_seq_search(&status)) != NULL) { @@ -421,10 +419,11 @@ MergeTable() if (old_table->role_table) { - InitRoleTableIfNeeded(); RoleEntry *entry; HASH_SEQ_STATUS status; + InitRoleTableIfNeeded(); + hash_seq_init(&status, old_table->role_table); while ((entry = hash_seq_search(&status)) != NULL) { @@ -515,9 +514,12 @@ RoleIsNeonSuperuser(const char *role_name) static void HandleCreateDb(CreatedbStmt *stmt) { - InitDbTableIfNeeded(); DefElem *downer = NULL; ListCell *option; + bool found = false; + DbEntry *entry; + + InitDbTableIfNeeded(); foreach(option, stmt->options) { @@ -526,13 +528,11 @@ HandleCreateDb(CreatedbStmt *stmt) if (strcmp(defel->defname, "owner") == 0) downer = defel; } - bool found = false; - DbEntry *entry = hash_search( - CurrentDdlTable->db_table, - stmt->dbname, - HASH_ENTER, - &found); + entry = hash_search(CurrentDdlTable->db_table, + stmt->dbname, + HASH_ENTER, + &found); if (!found) memset(entry->old_name, 0, sizeof(entry->old_name)); @@ -554,21 +554,24 @@ HandleCreateDb(CreatedbStmt *stmt) static void HandleAlterOwner(AlterOwnerStmt *stmt) { + const char *name; + bool found = false; + DbEntry *entry; + const char *new_owner; + if (stmt->objectType != OBJECT_DATABASE) return; InitDbTableIfNeeded(); - const char *name = strVal(stmt->object); - bool found = false; - DbEntry *entry = hash_search( - CurrentDdlTable->db_table, - name, - HASH_ENTER, - &found); + name = strVal(stmt->object); + entry = hash_search(CurrentDdlTable->db_table, + name, + HASH_ENTER, + &found); if (!found) memset(entry->old_name, 0, sizeof(entry->old_name)); - const char *new_owner = get_rolespec_name(stmt->newowner); + new_owner = get_rolespec_name(stmt->newowner); if (RoleIsNeonSuperuser(new_owner)) elog(ERROR, "can't alter owner to neon_superuser"); entry->owner = get_role_oid(new_owner, false); @@ -578,21 +581,23 @@ HandleAlterOwner(AlterOwnerStmt *stmt) static void HandleDbRename(RenameStmt *stmt) { + bool found = false; + DbEntry *entry; + DbEntry *entry_for_new_name; + Assert(stmt->renameType == OBJECT_DATABASE); InitDbTableIfNeeded(); - bool found = false; - DbEntry *entry = hash_search( - CurrentDdlTable->db_table, - stmt->subname, - HASH_FIND, - &found); - DbEntry *entry_for_new_name = hash_search( - CurrentDdlTable->db_table, - stmt->newname, - HASH_ENTER, - NULL); + entry = hash_search(CurrentDdlTable->db_table, + stmt->subname, + HASH_FIND, + &found); + entry_for_new_name = hash_search(CurrentDdlTable->db_table, + stmt->newname, + HASH_ENTER, + NULL); entry_for_new_name->type = Op_Set; + if (found) { if (entry->old_name[0] != '\0') @@ -600,8 +605,7 @@ HandleDbRename(RenameStmt *stmt) else strlcpy(entry_for_new_name->old_name, entry->name, NAMEDATALEN); entry_for_new_name->owner = entry->owner; - hash_search( - CurrentDdlTable->db_table, + hash_search(CurrentDdlTable->db_table, stmt->subname, HASH_REMOVE, NULL); @@ -616,14 +620,15 @@ HandleDbRename(RenameStmt *stmt) static void HandleDropDb(DropdbStmt *stmt) { - InitDbTableIfNeeded(); bool found = false; - DbEntry *entry = hash_search( - CurrentDdlTable->db_table, - stmt->dbname, - HASH_ENTER, - &found); + DbEntry *entry; + InitDbTableIfNeeded(); + + entry = hash_search(CurrentDdlTable->db_table, + stmt->dbname, + HASH_ENTER, + &found); entry->type = Op_Delete; entry->owner = InvalidOid; if (!found) @@ -633,16 +638,14 @@ HandleDropDb(DropdbStmt *stmt) static void HandleCreateRole(CreateRoleStmt *stmt) { - InitRoleTableIfNeeded(); bool found = false; - RoleEntry *entry = hash_search( - CurrentDdlTable->role_table, - stmt->role, - HASH_ENTER, - &found); - DefElem *dpass = NULL; + RoleEntry *entry; + DefElem *dpass; ListCell *option; + InitRoleTableIfNeeded(); + + dpass = NULL; foreach(option, stmt->options) { DefElem *defel = lfirst(option); @@ -650,6 +653,11 @@ HandleCreateRole(CreateRoleStmt *stmt) if (strcmp(defel->defname, "password") == 0) dpass = defel; } + + entry = hash_search(CurrentDdlTable->role_table, + stmt->role, + HASH_ENTER, + &found); if (!found) memset(entry->old_name, 0, sizeof(entry->old_name)); if (dpass && dpass->arg) @@ -662,14 +670,18 @@ HandleCreateRole(CreateRoleStmt *stmt) static void HandleAlterRole(AlterRoleStmt *stmt) { - InitRoleTableIfNeeded(); - DefElem *dpass = NULL; - ListCell *option; const char *role_name = stmt->role->rolename; + DefElem *dpass; + ListCell *option; + bool found = false; + RoleEntry *entry; + + InitRoleTableIfNeeded(); if (RoleIsNeonSuperuser(role_name) && !superuser()) elog(ERROR, "can't ALTER neon_superuser"); + dpass = NULL; foreach(option, stmt->options) { DefElem *defel = lfirst(option); @@ -680,13 +692,11 @@ HandleAlterRole(AlterRoleStmt *stmt) /* We only care about updates to the password */ if (!dpass) return; - bool found = false; - RoleEntry *entry = hash_search( - CurrentDdlTable->role_table, - role_name, - HASH_ENTER, - &found); + entry = hash_search(CurrentDdlTable->role_table, + role_name, + HASH_ENTER, + &found); if (!found) memset(entry->old_name, 0, sizeof(entry->old_name)); if (dpass->arg) @@ -699,20 +709,22 @@ HandleAlterRole(AlterRoleStmt *stmt) static void HandleRoleRename(RenameStmt *stmt) { - InitRoleTableIfNeeded(); - Assert(stmt->renameType == OBJECT_ROLE); bool found = false; - RoleEntry *entry = hash_search( - CurrentDdlTable->role_table, - stmt->subname, - HASH_FIND, - &found); + RoleEntry *entry; + RoleEntry *entry_for_new_name; - RoleEntry *entry_for_new_name = hash_search( - CurrentDdlTable->role_table, - stmt->newname, - HASH_ENTER, - NULL); + Assert(stmt->renameType == OBJECT_ROLE); + InitRoleTableIfNeeded(); + + entry = hash_search(CurrentDdlTable->role_table, + stmt->subname, + HASH_FIND, + &found); + + entry_for_new_name = hash_search(CurrentDdlTable->role_table, + stmt->newname, + HASH_ENTER, + NULL); entry_for_new_name->type = Op_Set; if (found) @@ -738,9 +750,10 @@ HandleRoleRename(RenameStmt *stmt) static void HandleDropRole(DropRoleStmt *stmt) { - InitRoleTableIfNeeded(); ListCell *item; + InitRoleTableIfNeeded(); + foreach(item, stmt->roles) { RoleSpec *spec = lfirst(item); diff --git a/pgxn/neon/file_cache.c b/pgxn/neon/file_cache.c index a8fc9ee312..92555a6bad 100644 --- a/pgxn/neon/file_cache.c +++ b/pgxn/neon/file_cache.c @@ -170,12 +170,14 @@ lfc_disable(char const *op) if (lfc_desc > 0) { + int rc; + /* * If the reason of error is ENOSPC, then truncation of file may * help to reclaim some space */ pgstat_report_wait_start(WAIT_EVENT_NEON_LFC_TRUNCATE); - int rc = ftruncate(lfc_desc, 0); + rc = ftruncate(lfc_desc, 0); pgstat_report_wait_end(); if (rc < 0) diff --git a/pgxn/neon/pagestore_smgr.c b/pgxn/neon/pagestore_smgr.c index 741d6bb3b8..7921218516 100644 --- a/pgxn/neon/pagestore_smgr.c +++ b/pgxn/neon/pagestore_smgr.c @@ -3747,6 +3747,8 @@ neon_read_slru_segment(SMgrRelation reln, const char* path, int segno, void* buf SlruKind kind; int n_blocks; shardno_t shard_no = 0; /* All SLRUs are at shard 0 */ + NeonResponse *resp; + NeonGetSlruSegmentRequest request; /* * Compute a request LSN to use, similar to neon_get_request_lsns() but the @@ -3785,8 +3787,7 @@ neon_read_slru_segment(SMgrRelation reln, const char* path, int segno, void* buf else return -1; - NeonResponse *resp; - NeonGetSlruSegmentRequest request = { + request = (NeonGetSlruSegmentRequest) { .req.tag = T_NeonGetSlruSegmentRequest, .req.lsn = request_lsn, .req.not_modified_since = not_modified_since, diff --git a/pgxn/neon/walproposer_pg.c b/pgxn/neon/walproposer_pg.c index 218277c7ed..706941c3f0 100644 --- a/pgxn/neon/walproposer_pg.c +++ b/pgxn/neon/walproposer_pg.c @@ -286,6 +286,9 @@ safekeepers_cmp(char *old, char *new) static void assign_neon_safekeepers(const char *newval, void *extra) { + char *newval_copy; + char *oldval; + if (!am_walproposer) return; @@ -295,8 +298,8 @@ assign_neon_safekeepers(const char *newval, void *extra) } /* Copy values because we will modify them in split_safekeepers_list() */ - char *newval_copy = pstrdup(newval); - char *oldval = pstrdup(wal_acceptors_list); + newval_copy = pstrdup(newval); + oldval = pstrdup(wal_acceptors_list); /* * TODO: restarting through FATAL is stupid and introduces 1s delay before diff --git a/pgxn/neon_walredo/walredoproc.c b/pgxn/neon_walredo/walredoproc.c index 4487fa0140..37abb3fa03 100644 --- a/pgxn/neon_walredo/walredoproc.c +++ b/pgxn/neon_walredo/walredoproc.c @@ -170,6 +170,40 @@ close_range_syscall(unsigned int start_fd, unsigned int count, unsigned int flag return syscall(__NR_close_range, start_fd, count, flags); } + +static PgSeccompRule allowed_syscalls[] = +{ + /* Hard requirements */ + PG_SCMP_ALLOW(exit_group), + PG_SCMP_ALLOW(pselect6), + PG_SCMP_ALLOW(read), + PG_SCMP_ALLOW(select), + PG_SCMP_ALLOW(write), + + /* Memory allocation */ + PG_SCMP_ALLOW(brk), +#ifndef MALLOC_NO_MMAP + /* TODO: musl doesn't have mallopt */ + PG_SCMP_ALLOW(mmap), + PG_SCMP_ALLOW(munmap), +#endif + /* + * getpid() is called on assertion failure, in ExceptionalCondition. + * It's not really needed, but seems pointless to hide it either. The + * system call unlikely to expose a kernel vulnerability, and the PID + * is stored in MyProcPid anyway. + */ + PG_SCMP_ALLOW(getpid), + + /* Enable those for a proper shutdown. */ +#if 0 + PG_SCMP_ALLOW(munmap), + PG_SCMP_ALLOW(shmctl), + PG_SCMP_ALLOW(shmdt), + PG_SCMP_ALLOW(unlink), /* shm_unlink */ +#endif +}; + static void enter_seccomp_mode(void) { @@ -183,44 +217,12 @@ enter_seccomp_mode(void) (errcode(ERRCODE_SYSTEM_ERROR), errmsg("seccomp: could not close files >= fd 3"))); - PgSeccompRule syscalls[] = - { - /* Hard requirements */ - PG_SCMP_ALLOW(exit_group), - PG_SCMP_ALLOW(pselect6), - PG_SCMP_ALLOW(read), - PG_SCMP_ALLOW(select), - PG_SCMP_ALLOW(write), - - /* Memory allocation */ - PG_SCMP_ALLOW(brk), -#ifndef MALLOC_NO_MMAP - /* TODO: musl doesn't have mallopt */ - PG_SCMP_ALLOW(mmap), - PG_SCMP_ALLOW(munmap), -#endif - /* - * getpid() is called on assertion failure, in ExceptionalCondition. - * It's not really needed, but seems pointless to hide it either. The - * system call unlikely to expose a kernel vulnerability, and the PID - * is stored in MyProcPid anyway. - */ - PG_SCMP_ALLOW(getpid), - - /* Enable those for a proper shutdown. - PG_SCMP_ALLOW(munmap), - PG_SCMP_ALLOW(shmctl), - PG_SCMP_ALLOW(shmdt), - PG_SCMP_ALLOW(unlink), // shm_unlink - */ - }; - #ifdef MALLOC_NO_MMAP /* Ask glibc not to use mmap() */ mallopt(M_MMAP_MAX, 0); #endif - seccomp_load_rules(syscalls, lengthof(syscalls)); + seccomp_load_rules(allowed_syscalls, lengthof(allowed_syscalls)); } #endif /* HAVE_LIBSECCOMP */