public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Eisentraut <[email protected]>
To: pgsql-hackers <[email protected]>
Subject: Fix inappropriate uses of atol()
Date: Sat, 3 Aug 2024 13:04:04 +0200
Message-ID: <[email protected]> (raw)
I noticed (during [0]) to some uses of the function atol() seem
inappropriate. Either they assume that sizeof(long)==8 and so might
truncate data if not, or they are gratuitous because the surrounding
code does not use the long type. This patch fixes these, by using
atoll() or atoi() instead. (There are still some atol() calls left
after this, which seemed ok to me.)
In the past, Windows didn't have atoll(), but the online documentation
appears to indicate that this now works in VS 2015 and later, which is
what we support at the moment. The Cirrus CI passes.
[0]:
https://www.postgresql.org/message-id/flat/[email protected]
From 5b3908bffd5f2731550c810dadb50e7d40f1b855 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Sat, 3 Aug 2024 12:29:50 +0200
Subject: [PATCH] Fix inappropriate uses of atol()
Some code using atol() would not work correctly if sizeof(long)==4:
- src/bin/pg_basebackup/pg_basebackup.c: Would miscount size of a
tablespace over 2 TB.
- src/bin/pg_basebackup/streamutil.c: Would truncate a timeline ID
beyond INT32_MAX.
- src/bin/pg_rewind/libpq_source.c: Would miscount files larger than 2
GB (but this currently cannot happen).
Replace these with atoll().
In a few cases, the use of atol() did not result in incorrect behavior
but seems inconsistent with related code:
- src/interfaces/ecpg/ecpglib/execute.c: Gratuitous, since it
processes a value from pg_type.typlen, which is int16.
- src/interfaces/ecpg/preproc/ecpg.trailer: Inconsistent, since the
rest of the preproc code uses atoi() for this.
Replace these with atoi().
---
src/bin/pg_basebackup/pg_basebackup.c | 2 +-
src/bin/pg_basebackup/streamutil.c | 2 +-
src/bin/pg_rewind/libpq_source.c | 2 +-
src/interfaces/ecpg/ecpglib/execute.c | 2 +-
src/interfaces/ecpg/preproc/ecpg.trailer | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8f3dd04fd22..12315442107 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -2056,7 +2056,7 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
tablespacecount = PQntuples(res);
for (i = 0; i < PQntuples(res); i++)
{
- totalsize_kb += atol(PQgetvalue(res, i, 2));
+ totalsize_kb += atoll(PQgetvalue(res, i, 2));
/*
* Verify tablespace directories are empty. Don't bother with the
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index feee451d595..30b3d9a377e 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -631,7 +631,7 @@ GetSlotInformation(PGconn *conn, const char *slot_name,
/* current TLI */
if (!PQgetisnull(res, 0, 2))
- tli_loc = (TimeLineID) atol(PQgetvalue(res, 0, 2));
+ tli_loc = (TimeLineID) atoll(PQgetvalue(res, 0, 2));
PQclear(res);
diff --git a/src/bin/pg_rewind/libpq_source.c b/src/bin/pg_rewind/libpq_source.c
index 9378266d28e..0239fb47644 100644
--- a/src/bin/pg_rewind/libpq_source.c
+++ b/src/bin/pg_rewind/libpq_source.c
@@ -294,7 +294,7 @@ libpq_traverse_files(rewind_source *source, process_file_callback_t callback)
}
path = PQgetvalue(res, i, 0);
- filesize = atol(PQgetvalue(res, i, 1));
+ filesize = atoll(PQgetvalue(res, i, 1));
isdir = (strcmp(PQgetvalue(res, i, 2), "t") == 0);
link_target = PQgetvalue(res, i, 3);
diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c
index 04d0b40c537..c578c21cf66 100644
--- a/src/interfaces/ecpg/ecpglib/execute.c
+++ b/src/interfaces/ecpg/ecpglib/execute.c
@@ -278,7 +278,7 @@ ecpg_is_type_an_array(int type, const struct statement *stmt, const struct varia
isarray = ECPG_ARRAY_NONE;
else
{
- isarray = (atol((char *) PQgetvalue(query, 0, 0)) == -1) ? ECPG_ARRAY_ARRAY : ECPG_ARRAY_VECTOR;
+ isarray = (atoi((char *) PQgetvalue(query, 0, 0)) == -1) ? ECPG_ARRAY_ARRAY : ECPG_ARRAY_VECTOR;
if (ecpg_dynamic_type(type) == SQL3_CHARACTER ||
ecpg_dynamic_type(type) == SQL3_CHARACTER_VARYING)
{
diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer b/src/interfaces/ecpg/preproc/ecpg.trailer
index b2aa44f36dd..8ac1c5c9eda 100644
--- a/src/interfaces/ecpg/preproc/ecpg.trailer
+++ b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -217,7 +217,7 @@ char_variable: cvariable
enum ECPGttype type = p->type->type;
/* If we have just one character this is not a string */
- if (atol(p->type->size) == 1)
+ if (atoi(p->type->size) == 1)
mmerror(PARSE_ERROR, ET_ERROR, "invalid data type");
else
{
base-commit: 66188912566b5614dff095ae86f4b1e06d58e875
--
2.45.2
Attachments:
[text/plain] 0001-Fix-inappropriate-uses-of-atol.patch (4.0K, ../[email protected]/2-0001-Fix-inappropriate-uses-of-atol.patch)
download | inline diff:
From 5b3908bffd5f2731550c810dadb50e7d40f1b855 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Sat, 3 Aug 2024 12:29:50 +0200
Subject: [PATCH] Fix inappropriate uses of atol()
Some code using atol() would not work correctly if sizeof(long)==4:
- src/bin/pg_basebackup/pg_basebackup.c: Would miscount size of a
tablespace over 2 TB.
- src/bin/pg_basebackup/streamutil.c: Would truncate a timeline ID
beyond INT32_MAX.
- src/bin/pg_rewind/libpq_source.c: Would miscount files larger than 2
GB (but this currently cannot happen).
Replace these with atoll().
In a few cases, the use of atol() did not result in incorrect behavior
but seems inconsistent with related code:
- src/interfaces/ecpg/ecpglib/execute.c: Gratuitous, since it
processes a value from pg_type.typlen, which is int16.
- src/interfaces/ecpg/preproc/ecpg.trailer: Inconsistent, since the
rest of the preproc code uses atoi() for this.
Replace these with atoi().
---
src/bin/pg_basebackup/pg_basebackup.c | 2 +-
src/bin/pg_basebackup/streamutil.c | 2 +-
src/bin/pg_rewind/libpq_source.c | 2 +-
src/interfaces/ecpg/ecpglib/execute.c | 2 +-
src/interfaces/ecpg/preproc/ecpg.trailer | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8f3dd04fd22..12315442107 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -2056,7 +2056,7 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
tablespacecount = PQntuples(res);
for (i = 0; i < PQntuples(res); i++)
{
- totalsize_kb += atol(PQgetvalue(res, i, 2));
+ totalsize_kb += atoll(PQgetvalue(res, i, 2));
/*
* Verify tablespace directories are empty. Don't bother with the
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index feee451d595..30b3d9a377e 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -631,7 +631,7 @@ GetSlotInformation(PGconn *conn, const char *slot_name,
/* current TLI */
if (!PQgetisnull(res, 0, 2))
- tli_loc = (TimeLineID) atol(PQgetvalue(res, 0, 2));
+ tli_loc = (TimeLineID) atoll(PQgetvalue(res, 0, 2));
PQclear(res);
diff --git a/src/bin/pg_rewind/libpq_source.c b/src/bin/pg_rewind/libpq_source.c
index 9378266d28e..0239fb47644 100644
--- a/src/bin/pg_rewind/libpq_source.c
+++ b/src/bin/pg_rewind/libpq_source.c
@@ -294,7 +294,7 @@ libpq_traverse_files(rewind_source *source, process_file_callback_t callback)
}
path = PQgetvalue(res, i, 0);
- filesize = atol(PQgetvalue(res, i, 1));
+ filesize = atoll(PQgetvalue(res, i, 1));
isdir = (strcmp(PQgetvalue(res, i, 2), "t") == 0);
link_target = PQgetvalue(res, i, 3);
diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c
index 04d0b40c537..c578c21cf66 100644
--- a/src/interfaces/ecpg/ecpglib/execute.c
+++ b/src/interfaces/ecpg/ecpglib/execute.c
@@ -278,7 +278,7 @@ ecpg_is_type_an_array(int type, const struct statement *stmt, const struct varia
isarray = ECPG_ARRAY_NONE;
else
{
- isarray = (atol((char *) PQgetvalue(query, 0, 0)) == -1) ? ECPG_ARRAY_ARRAY : ECPG_ARRAY_VECTOR;
+ isarray = (atoi((char *) PQgetvalue(query, 0, 0)) == -1) ? ECPG_ARRAY_ARRAY : ECPG_ARRAY_VECTOR;
if (ecpg_dynamic_type(type) == SQL3_CHARACTER ||
ecpg_dynamic_type(type) == SQL3_CHARACTER_VARYING)
{
diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer b/src/interfaces/ecpg/preproc/ecpg.trailer
index b2aa44f36dd..8ac1c5c9eda 100644
--- a/src/interfaces/ecpg/preproc/ecpg.trailer
+++ b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -217,7 +217,7 @@ char_variable: cvariable
enum ECPGttype type = p->type->type;
/* If we have just one character this is not a string */
- if (atol(p->type->size) == 1)
+ if (atoi(p->type->size) == 1)
mmerror(PARSE_ERROR, ET_ERROR, "invalid data type");
else
{
base-commit: 66188912566b5614dff095ae86f4b1e06d58e875
--
2.45.2
view thread (2+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: Fix inappropriate uses of atol()
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox