Re: pg_upgrade - Mailing list pgsql-general

Bruce Momjian wrote:
> Brian Hirt wrote:
> > It looks like it's related to atol
>
> Yep, I found the use of atol in the pg_upgrade code too.  Working on a
> patch now.

I have applied the attached patch to HEAD and 9.0.X.  Odd I had never
received a bug report about this before.  Good thing it didn't silently
fail, but it is designed to be very picky.

This patch will appear in the next 9.0.X release.  Thanks for the
report.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/controldata.c b/contrib/pg_upgrade/controldata.c
index f36c2c1..c0fe821 100644
--- a/contrib/pg_upgrade/controldata.c
+++ b/contrib/pg_upgrade/controldata.c
@@ -155,7 +155,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: pg_resetxlog problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.ctrl_ver = (uint32) atol(p);
+            cluster->controldata.ctrl_ver = str2uint(p);
         }
         else if ((p = strstr(bufin, "Catalog version number:")) != NULL)
         {
@@ -165,7 +165,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.cat_ver = (uint32) atol(p);
+            cluster->controldata.cat_ver = str2uint(p);
         }
         else if ((p = strstr(bufin, "First log file ID after reset:")) != NULL)
         {
@@ -175,7 +175,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.logid = (uint32) atol(p);
+            cluster->controldata.logid = str2uint(p);
             got_log_id = true;
         }
         else if ((p = strstr(bufin, "First log file segment after reset:")) != NULL)
@@ -186,7 +186,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.nxtlogseg = (uint32) atol(p);
+            cluster->controldata.nxtlogseg = str2uint(p);
             got_log_seg = true;
         }
         else if ((p = strstr(bufin, "Latest checkpoint's TimeLineID:")) != NULL)
@@ -197,7 +197,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.chkpnt_tli = (uint32) atol(p);
+            cluster->controldata.chkpnt_tli = str2uint(p);
             got_tli = true;
         }
         else if ((p = strstr(bufin, "Latest checkpoint's NextXID:")) != NULL)
@@ -211,7 +211,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             op++;                /* removing ':' char */
-            cluster->controldata.chkpnt_nxtxid = (uint32) atol(op);
+            cluster->controldata.chkpnt_nxtxid = str2uint(op);
             got_xid = true;
         }
         else if ((p = strstr(bufin, "Latest checkpoint's NextOID:")) != NULL)
@@ -222,7 +222,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.chkpnt_nxtoid = (uint32) atol(p);
+            cluster->controldata.chkpnt_nxtoid = str2uint(p);
             got_oid = true;
         }
         else if ((p = strstr(bufin, "Maximum data alignment:")) != NULL)
@@ -233,7 +233,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.align = (uint32) atol(p);
+            cluster->controldata.align = str2uint(p);
             got_align = true;
         }
         else if ((p = strstr(bufin, "Database block size:")) != NULL)
@@ -244,7 +244,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.blocksz = (uint32) atol(p);
+            cluster->controldata.blocksz = str2uint(p);
             got_blocksz = true;
         }
         else if ((p = strstr(bufin, "Blocks per segment of large relation:")) != NULL)
@@ -255,7 +255,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.largesz = (uint32) atol(p);
+            cluster->controldata.largesz = str2uint(p);
             got_largesz = true;
         }
         else if ((p = strstr(bufin, "WAL block size:")) != NULL)
@@ -266,7 +266,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.walsz = (uint32) atol(p);
+            cluster->controldata.walsz = str2uint(p);
             got_walsz = true;
         }
         else if ((p = strstr(bufin, "Bytes per WAL segment:")) != NULL)
@@ -277,7 +277,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.walseg = (uint32) atol(p);
+            cluster->controldata.walseg = str2uint(p);
             got_walseg = true;
         }
         else if ((p = strstr(bufin, "Maximum length of identifiers:")) != NULL)
@@ -288,7 +288,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.ident = (uint32) atol(p);
+            cluster->controldata.ident = str2uint(p);
             got_ident = true;
         }
         else if ((p = strstr(bufin, "Maximum columns in an index:")) != NULL)
@@ -299,7 +299,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.index = (uint32) atol(p);
+            cluster->controldata.index = str2uint(p);
             got_index = true;
         }
         else if ((p = strstr(bufin, "Maximum size of a TOAST chunk:")) != NULL)
@@ -310,7 +310,7 @@ get_control_data(migratorContext *ctx, ClusterInfo *cluster, bool live_check)
                 pg_log(ctx, PG_FATAL, "%d: controldata retrieval problem\n", __LINE__);

             p++;                /* removing ':' char */
-            cluster->controldata.toast = (uint32) atol(p);
+            cluster->controldata.toast = str2uint(p);
             got_toast = true;
         }
         else if ((p = strstr(bufin, "Date/time type storage:")) != NULL)
diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c
index 1601d56..55489ac 100644
--- a/contrib/pg_upgrade/info.c
+++ b/contrib/pg_upgrade/info.c
@@ -242,7 +242,7 @@ get_db_infos(migratorContext *ctx, DbInfoArr *dbinfs_arr, Cluster whichCluster)

     for (tupnum = 0; tupnum < ntups; tupnum++)
     {
-        dbinfos[tupnum].db_oid = atol(PQgetvalue(res, tupnum, i_oid));
+        dbinfos[tupnum].db_oid = str2uint(PQgetvalue(res, tupnum, i_oid));

         snprintf(dbinfos[tupnum].db_name, sizeof(dbinfos[tupnum].db_name), "%s",
                  PQgetvalue(res, tupnum, i_datname));
@@ -360,7 +360,7 @@ get_rel_infos(migratorContext *ctx, const DbInfo *dbinfo,
         RelInfo    *curr = &relinfos[num_rels++];
         const char *tblspace;

-        curr->reloid = atol(PQgetvalue(res, relnum, i_oid));
+        curr->reloid = str2uint(PQgetvalue(res, relnum, i_oid));

         nspname = PQgetvalue(res, relnum, i_nspname);
         strlcpy(curr->nspname, nspname, sizeof(curr->nspname));
@@ -368,8 +368,8 @@ get_rel_infos(migratorContext *ctx, const DbInfo *dbinfo,
         relname = PQgetvalue(res, relnum, i_relname);
         strlcpy(curr->relname, relname, sizeof(curr->relname));

-        curr->relfilenode = atol(PQgetvalue(res, relnum, i_relfilenode));
-        curr->toastrelid = atol(PQgetvalue(res, relnum, i_reltoastrelid));
+        curr->relfilenode = str2uint(PQgetvalue(res, relnum, i_relfilenode));
+        curr->toastrelid = str2uint(PQgetvalue(res, relnum, i_reltoastrelid));

         tblspace = PQgetvalue(res, relnum, i_spclocation);
         /* if no table tablespace, use the database tablespace */
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
index a7d46cf..296b80f 100644
--- a/contrib/pg_upgrade/pg_upgrade.h
+++ b/contrib/pg_upgrade/pg_upgrade.h
@@ -376,6 +376,8 @@ char       *pg_strdup(migratorContext *ctx, const char *s);
 void       *pg_malloc(migratorContext *ctx, int size);
 void        pg_free(void *ptr);
 const char *getErrorText(int errNum);
+unsigned int str2uint(const char *str);
+

 /* version.c */

diff --git a/contrib/pg_upgrade/relfilenode.c b/contrib/pg_upgrade/relfilenode.c
index a69548b..dd605bb 100644
--- a/contrib/pg_upgrade/relfilenode.c
+++ b/contrib/pg_upgrade/relfilenode.c
@@ -94,9 +94,9 @@ get_pg_database_relfilenode(migratorContext *ctx, Cluster whichCluster)

     i_relfile = PQfnumber(res, "relfilenode");
     if (whichCluster == CLUSTER_OLD)
-        ctx->old.pg_database_oid = atol(PQgetvalue(res, 0, i_relfile));
+        ctx->old.pg_database_oid = str2uint(PQgetvalue(res, 0, i_relfile));
     else
-        ctx->new.pg_database_oid = atol(PQgetvalue(res, 0, i_relfile));
+        ctx->new.pg_database_oid = str2uint(PQgetvalue(res, 0, i_relfile));

     PQclear(res);
     PQfinish(conn);
diff --git a/contrib/pg_upgrade/util.c b/contrib/pg_upgrade/util.c
index b9968e9..3f3a4c7 100644
--- a/contrib/pg_upgrade/util.c
+++ b/contrib/pg_upgrade/util.c
@@ -259,3 +259,15 @@ getErrorText(int errNum)
 #endif
     return strdup(strerror(errNum));
 }
+
+
+/*
+ *    str2uint()
+ *
+ *    convert string to oid
+ */
+unsigned int
+str2uint(const char *str)
+{
+    return strtol(str, NULL, 10);
+}