]> granicus.if.org Git - postgresql/commitdiff
Avoid portability issues in autoprewarm.c.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 3 May 2018 16:50:27 +0000 (12:50 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 3 May 2018 16:50:34 +0000 (12:50 -0400)
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

index 9faabe576dd87fadb0480ef81d5a230f9cda54de..cc5e2dd89cd48de64d5e9dff6f10104f0596aad7 100644 (file)
@@ -25,6 +25,7 @@
  */
 
 #include "postgres.h"
+
 #include <unistd.h>
 
 #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);