From cddc4dc6c6c9bc6f9e8e7f81b5264e7265a5c070 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 3 May 2018 12:50:27 -0400 Subject: [PATCH] Avoid portability issues in autoprewarm.c. autoprewarm.c mostly considered the number of blocks it might be dealing with as being int64. This is unnecessary, because NBuffers is declared as int, and there's been no suggestion that we might widen it in the foreseeable future. Moreover, using int64 is problematic because the code expected INT64_FORMAT to work with fscanf(), something we don't guarantee, and which indeed fails on some older buildfarm members. On top of that, the module randomly used uint32 rather than int64 variables to hold block counters in several places, so it would fail anyway if we ever did have NBuffers wider than that; and it also supposed that pg_qsort could sort an int64 number of elements, which is wrong on 32-bit machines (though no doubt a 32-bit machine couldn't actually have that many buffers). Hence, change all these variables to plain int. In passing, avoid shadowing one variable named i with another, and avoid casting away const in apw_compare_blockinfo. Discussion: https://postgr.es/m/7773.1525288909@sss.pgh.pa.us --- contrib/pg_prewarm/autoprewarm.c | 61 +++++++++++++++++--------------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index 9faabe576d..cc5e2dd89c 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -25,6 +25,7 @@ */ #include "postgres.h" + #include #include "access/heapam.h" @@ -73,9 +74,9 @@ typedef struct AutoPrewarmSharedState /* Following items are for communication with per-database worker */ dsm_handle block_info_handle; Oid database; - int64 prewarm_start_idx; - int64 prewarm_stop_idx; - int64 prewarmed_blocks; + int prewarm_start_idx; + int prewarm_stop_idx; + int prewarmed_blocks; } AutoPrewarmSharedState; void _PG_init(void); @@ -86,7 +87,7 @@ PG_FUNCTION_INFO_V1(autoprewarm_start_worker); PG_FUNCTION_INFO_V1(autoprewarm_dump_now); static void apw_load_buffers(void); -static int64 apw_dump_now(bool is_bgworker, bool dump_unlogged); +static int apw_dump_now(bool is_bgworker, bool dump_unlogged); static void apw_start_master_worker(void); static void apw_start_database_worker(void); static bool apw_init_shmem(void); @@ -274,7 +275,7 @@ static void apw_load_buffers(void) { FILE *file = NULL; - int64 num_elements, + int num_elements, i; BlockInfoRecord *blkinfo; dsm_segment *seg; @@ -317,7 +318,7 @@ apw_load_buffers(void) } /* First line of the file is a record count. */ - if (fscanf(file, "<<" INT64_FORMAT ">>\n", &num_elements) != 1) + if (fscanf(file, "<<%d>>\n", &num_elements) != 1) ereport(ERROR, (errcode_for_file_access(), errmsg("could not read from file \"%s\": %m", @@ -336,7 +337,7 @@ apw_load_buffers(void) &blkinfo[i].tablespace, &blkinfo[i].filenode, &forknum, &blkinfo[i].blocknum) != 5) ereport(ERROR, - (errmsg("autoprewarm block dump file is corrupted at line " INT64_FORMAT, + (errmsg("autoprewarm block dump file is corrupted at line %d", i + 1))); blkinfo[i].forknum = forknum; } @@ -355,17 +356,17 @@ apw_load_buffers(void) /* Get the info position of the first block of the next database. */ while (apw_state->prewarm_start_idx < num_elements) { - uint32 i = apw_state->prewarm_start_idx; - Oid current_db = blkinfo[i].database; + int j = apw_state->prewarm_start_idx; + Oid current_db = blkinfo[j].database; /* * Advance the prewarm_stop_idx to the first BlockRecordInfo that does * not belong to this database. */ - i++; - while (i < num_elements) + j++; + while (j < num_elements) { - if (current_db != blkinfo[i].database) + if (current_db != blkinfo[j].database) { /* * Combine BlockRecordInfos for global objects with those of @@ -373,10 +374,10 @@ apw_load_buffers(void) */ if (current_db != InvalidOid) break; - current_db = blkinfo[i].database; + current_db = blkinfo[j].database; } - i++; + j++; } /* @@ -388,7 +389,7 @@ apw_load_buffers(void) break; /* Configure stop point and database for next per-database worker. */ - apw_state->prewarm_stop_idx = i; + apw_state->prewarm_stop_idx = j; apw_state->database = current_db; Assert(apw_state->prewarm_start_idx < apw_state->prewarm_stop_idx); @@ -415,8 +416,7 @@ apw_load_buffers(void) /* Report our success. */ ereport(LOG, - (errmsg("autoprewarm successfully prewarmed " INT64_FORMAT - " of " INT64_FORMAT " previously-loaded blocks", + (errmsg("autoprewarm successfully prewarmed %d of %d previously-loaded blocks", apw_state->prewarmed_blocks, num_elements))); } @@ -427,7 +427,7 @@ apw_load_buffers(void) void autoprewarm_database_main(Datum main_arg) { - uint32 pos; + int pos; BlockInfoRecord *block_info; Relation rel = NULL; BlockNumber nblocks = 0; @@ -557,13 +557,14 @@ autoprewarm_database_main(Datum main_arg) * Dump information on blocks in shared buffers. We use a text format here * so that it's easy to understand and even change the file contents if * necessary. + * Returns the number of blocks dumped. */ -static int64 +static int apw_dump_now(bool is_bgworker, bool dump_unlogged) { - uint32 i; + int num_blocks; + int i; int ret; - int64 num_blocks; BlockInfoRecord *block_info_array; BufferDesc *bufHdr; FILE *file; @@ -630,7 +631,7 @@ apw_dump_now(bool is_bgworker, bool dump_unlogged) errmsg("could not open file \"%s\": %m", transient_dump_file_path))); - ret = fprintf(file, "<<" INT64_FORMAT ">>\n", num_blocks); + ret = fprintf(file, "<<%d>>\n", num_blocks); if (ret < 0) { int save_errno = errno; @@ -691,8 +692,7 @@ apw_dump_now(bool is_bgworker, bool dump_unlogged) apw_state->pid_using_dumpfile = InvalidPid; ereport(DEBUG1, - (errmsg("wrote block details for " INT64_FORMAT " blocks", - num_blocks))); + (errmsg("wrote block details for %d blocks", num_blocks))); return num_blocks; } @@ -727,11 +727,14 @@ autoprewarm_start_worker(PG_FUNCTION_ARGS) /* * SQL-callable function to perform an immediate block dump. + * + * Note: this is declared to return int8, as insurance against some + * very distant day when we might make NBuffers wider than int. */ Datum autoprewarm_dump_now(PG_FUNCTION_ARGS) { - int64 num_blocks; + int num_blocks; apw_init_shmem(); @@ -741,7 +744,7 @@ autoprewarm_dump_now(PG_FUNCTION_ARGS) } PG_END_ENSURE_ERROR_CLEANUP(apw_detach_shmem, 0); - PG_RETURN_INT64(num_blocks); + PG_RETURN_INT64((int64) num_blocks); } /* @@ -869,7 +872,7 @@ do { \ return -1; \ else if (a->fld > b->fld) \ return 1; \ -} while(0); +} while(0) /* * apw_compare_blockinfo @@ -883,8 +886,8 @@ do { \ static int apw_compare_blockinfo(const void *p, const void *q) { - BlockInfoRecord *a = (BlockInfoRecord *) p; - BlockInfoRecord *b = (BlockInfoRecord *) q; + const BlockInfoRecord *a = (const BlockInfoRecord *) p; + const BlockInfoRecord *b = (const BlockInfoRecord *) q; cmp_member_elem(database); cmp_member_elem(tablespace);