public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v17 04/10] Add pg_ls_dir_metadata to list a dir with file metadata.. 2+ messages / 2 participants [nested] [flat]
* [PATCH v17 04/10] Add pg_ls_dir_metadata to list a dir with file metadata.. @ 2020-03-10 03:40 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Justin Pryzby @ 2020-03-10 03:40 UTC (permalink / raw) Generalize pg_ls_dir_files and retire pg_ls_dir Need catversion bumped? --- doc/src/sgml/func.sgml | 21 ++ src/backend/catalog/system_views.sql | 1 + src/backend/utils/adt/genfile.c | 229 +++++++++++-------- src/include/catalog/pg_proc.dat | 12 + src/test/regress/expected/misc_functions.out | 24 ++ src/test/regress/input/tablespace.source | 5 + src/test/regress/output/tablespace.source | 8 + src/test/regress/sql/misc_functions.sql | 11 + 8 files changed, 220 insertions(+), 91 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index d9b3598977..fc1b4ac98c 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -25714,6 +25714,27 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size </para></entry> </row> + <row> + <entry role="func_table_entry"><para role="func_signature"> + <indexterm> + <primary>pg_ls_dir_metadata</primary> + </indexterm> + <function>pg_ls_dir_metadata</function> ( <parameter>dirname</parameter> <type>text</type> + <optional>, <parameter>missing_ok</parameter> <type>boolean</type>, + <parameter>include_dot_dirs</parameter> <type>boolean</type>] </optional> ) + <returnvalue>setof record</returnvalue> + ( <parameter>name</parameter> <type>text</type>, + <parameter>size</parameter> <type>bigint</type>, + <parameter>modification</parameter> <type>timestamp with time zone</type> ) + </para> + <para> + For each file in the specified directory, list the file and its + metadata. + Restricted to superusers by default, but other users can be granted + EXECUTE to run the function. + </para></entry> + </row> + <row> <entry role="func_table_entry"><para role="func_signature"> <indexterm> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 2bd5f5ea14..1c77430f0c 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1467,6 +1467,7 @@ REVOKE EXECUTE ON FUNCTION pg_stat_file(text,boolean) FROM public; REVOKE EXECUTE ON FUNCTION pg_ls_dir(text) FROM public; REVOKE EXECUTE ON FUNCTION pg_ls_dir(text,boolean,boolean) FROM public; +REVOKE EXECUTE ON FUNCTION pg_ls_dir_metadata(text,boolean,boolean) FROM public; -- -- We also set up some things as accessible to standard roles. diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c index 219ac160f8..4824a55480 100644 --- a/src/backend/utils/adt/genfile.c +++ b/src/backend/utils/adt/genfile.c @@ -36,6 +36,21 @@ #include "utils/syscache.h" #include "utils/timestamp.h" +static Datum pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags); + +#define LS_DIR_ISDIR (1<<0) /* Show column: isdir */ +#define LS_DIR_METADATA (1<<1) /* Show columns: mtime, size */ +#define LS_DIR_MISSING_OK (1<<2) /* Ignore ENOENT if the toplevel dir is missing */ +#define LS_DIR_SKIP_DOT_DIRS (1<<3) /* Do not show . or .. */ +#define LS_DIR_SKIP_HIDDEN (1<<4) /* Do not show anything begining with . */ +#define LS_DIR_SKIP_DIRS (1<<5) /* Do not show directories */ +#define LS_DIR_SKIP_SPECIAL (1<<6) /* Do not show special file types */ + +/* + * Shortcut for the historic behavior of the pg_ls_* functions (not including + * pg_ls_dir, which skips different files and doesn't show metadata. + */ +#define LS_DIR_HISTORIC (LS_DIR_SKIP_DIRS|LS_DIR_SKIP_HIDDEN|LS_DIR_SKIP_SPECIAL|LS_DIR_METADATA) /* * Convert a "text" filename argument to C string, and check it's allowable. @@ -413,6 +428,11 @@ pg_stat_file(PG_FUNCTION_ARGS) values[4] = TimestampTzGetDatum(time_t_to_timestamptz(fst.st_ctime)); #endif values[5] = BoolGetDatum(S_ISDIR(fst.st_mode)); +#ifdef WIN32 + /* Links should have isdir=false */ + if (pgwin32_is_junction(filename)) + values[5] = BoolGetDatum(false); +#endif tuple = heap_form_tuple(tupdesc, values, isnull); @@ -440,79 +460,9 @@ pg_stat_file_1arg(PG_FUNCTION_ARGS) Datum pg_ls_dir(PG_FUNCTION_ARGS) { - ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; - char *location; - bool missing_ok = false; - bool include_dot_dirs = false; - bool randomAccess; - TupleDesc tupdesc; - Tuplestorestate *tupstore; - DIR *dirdesc; - struct dirent *de; - MemoryContext oldcontext; - - location = convert_and_check_filename(PG_GETARG_TEXT_PP(0)); - - /* check the optional arguments */ - if (PG_NARGS() == 3) - { - if (!PG_ARGISNULL(1)) - missing_ok = PG_GETARG_BOOL(1); - if (!PG_ARGISNULL(2)) - include_dot_dirs = PG_GETARG_BOOL(2); - } - - /* check to see if caller supports us returning a tuplestore */ - if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("set-valued function called in context that cannot accept a set"))); - if (!(rsinfo->allowedModes & SFRM_Materialize)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("materialize mode required, but it is not allowed in this context"))); - - /* The tupdesc and tuplestore must be created in ecxt_per_query_memory */ - oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory); - - tupdesc = CreateTemplateTupleDesc(1); - TupleDescInitEntry(tupdesc, (AttrNumber) 1, "pg_ls_dir", TEXTOID, -1, 0); - - randomAccess = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0; - tupstore = tuplestore_begin_heap(randomAccess, false, work_mem); - rsinfo->returnMode = SFRM_Materialize; - rsinfo->setResult = tupstore; - rsinfo->setDesc = tupdesc; - - MemoryContextSwitchTo(oldcontext); - - dirdesc = AllocateDir(location); - if (!dirdesc) - { - /* Return empty tuplestore if appropriate */ - if (missing_ok && errno == ENOENT) - return (Datum) 0; - /* Otherwise, we can let ReadDir() throw the error */ - } - - while ((de = ReadDir(dirdesc, location)) != NULL) - { - Datum values[1]; - bool nulls[1]; - - if (!include_dot_dirs && - (strcmp(de->d_name, ".") == 0 || - strcmp(de->d_name, "..") == 0)) - continue; - - values[0] = CStringGetTextDatum(de->d_name); - nulls[0] = false; - - tuplestore_putvalues(tupstore, tupdesc, values, nulls); - } - - FreeDir(dirdesc); - return (Datum) 0; + text *filename_t = PG_GETARG_TEXT_PP(0); + char *filename = convert_and_check_filename(filename_t); + return pg_ls_dir_files(fcinfo, filename, LS_DIR_SKIP_DOT_DIRS); } /* @@ -525,7 +475,9 @@ pg_ls_dir(PG_FUNCTION_ARGS) Datum pg_ls_dir_1arg(PG_FUNCTION_ARGS) { - return pg_ls_dir(fcinfo); + text *filename_t = PG_GETARG_TEXT_PP(0); + char *filename = convert_and_check_filename(filename_t); + return pg_ls_dir_files(fcinfo, filename, LS_DIR_SKIP_DOT_DIRS); } /* @@ -535,7 +487,7 @@ pg_ls_dir_1arg(PG_FUNCTION_ARGS) * Other unreadable-directory cases throw an error. */ static Datum -pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok) +pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, int flags) { ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; bool randomAccess; @@ -544,6 +496,32 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok) DIR *dirdesc; struct dirent *de; MemoryContext oldcontext; + TypeFuncClass tuptype ; + + /* isdir depends on metadata */ + Assert(!(flags&LS_DIR_ISDIR) || (flags&LS_DIR_METADATA)); + /* Unreasonable to show isdir and skip dirs */ + Assert(!(flags&LS_DIR_ISDIR) || !(flags&LS_DIR_SKIP_DIRS)); + + /* check the optional arguments */ + if (PG_NARGS() == 3) + { + if (!PG_ARGISNULL(1)) + { + if (PG_GETARG_BOOL(1)) + flags |= LS_DIR_MISSING_OK; + else + flags &= ~LS_DIR_MISSING_OK; + } + + if (!PG_ARGISNULL(2)) + { + if (PG_GETARG_BOOL(2)) + flags &= ~LS_DIR_SKIP_DOT_DIRS; + else + flags |= LS_DIR_SKIP_DOT_DIRS; + } + } /* check to see if caller supports us returning a tuplestore */ if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo)) @@ -558,8 +536,20 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok) /* The tupdesc and tuplestore must be created in ecxt_per_query_memory */ oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory); - if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) - elog(ERROR, "return type must be a row type"); + tuptype = get_call_result_type(fcinfo, NULL, &tupdesc); + if (flags & LS_DIR_METADATA) + { + if (tuptype != TYPEFUNC_COMPOSITE) + elog(ERROR, "return type must be a row type"); + } + else + { + /* pg_ls_dir returns a simple scalar */ + if (tuptype != TYPEFUNC_SCALAR) + elog(ERROR, "return type must be a scalar type"); + tupdesc = CreateTemplateTupleDesc(1); + TupleDescInitEntry(tupdesc, (AttrNumber) 1, "column", TEXTOID, -1, 0); + } randomAccess = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0; tupstore = tuplestore_begin_heap(randomAccess, false, work_mem); @@ -578,20 +568,27 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok) if (!dirdesc) { /* Return empty tuplestore if appropriate */ - if (missing_ok && errno == ENOENT) + if (flags & LS_DIR_MISSING_OK && errno == ENOENT) return (Datum) 0; /* Otherwise, we can let ReadDir() throw the error */ } while ((de = ReadDir(dirdesc, dir)) != NULL) { - Datum values[3]; - bool nulls[3]; + Datum values[4]; + bool nulls[4]; char path[MAXPGPATH * 2]; struct stat attrib; - /* Skip hidden files */ - if (de->d_name[0] == '.') + /* Skip dot dirs? */ + if (flags & LS_DIR_SKIP_DOT_DIRS && + (strcmp(de->d_name, ".") == 0 || + strcmp(de->d_name, "..") == 0)) + continue; + + /* Skip hidden files? */ + if (flags & LS_DIR_SKIP_HIDDEN && + de->d_name[0] == '.') continue; /* Get the file info */ @@ -606,13 +603,34 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok) errmsg("could not stat file \"%s\": %m", path))); } - /* Ignore anything but regular files */ - if (!S_ISREG(attrib.st_mode)) - continue; + /* Skip dirs or special files? */ + if (S_ISDIR(attrib.st_mode)) + { + if (flags & LS_DIR_SKIP_DIRS) + continue; + } + else if (!S_ISREG(attrib.st_mode)) + { + if (flags & LS_DIR_SKIP_SPECIAL) + continue; + } values[0] = CStringGetTextDatum(de->d_name); - values[1] = Int64GetDatum((int64) attrib.st_size); - values[2] = TimestampTzGetDatum(time_t_to_timestamptz(attrib.st_mtime)); + if (flags & LS_DIR_METADATA) + { + values[1] = Int64GetDatum((int64) attrib.st_size); + values[2] = TimestampTzGetDatum(time_t_to_timestamptz(attrib.st_mtime)); + if (flags & LS_DIR_ISDIR) + { + values[3] = BoolGetDatum(S_ISDIR(attrib.st_mode)); +#ifdef WIN32 + /* Links should have isdir=false */ + if (pgwin32_is_junction(path)) + values[3] = BoolGetDatum(false); +#endif + } + } + memset(nulls, 0, sizeof(nulls)); tuplestore_putvalues(tupstore, tupdesc, values, nulls); @@ -626,14 +644,14 @@ pg_ls_dir_files(FunctionCallInfo fcinfo, const char *dir, bool missing_ok) Datum pg_ls_logdir(PG_FUNCTION_ARGS) { - return pg_ls_dir_files(fcinfo, Log_directory, false); + return pg_ls_dir_files(fcinfo, Log_directory, LS_DIR_HISTORIC); } /* Function to return the list of files in the WAL directory */ Datum pg_ls_waldir(PG_FUNCTION_ARGS) { - return pg_ls_dir_files(fcinfo, XLOGDIR, false); + return pg_ls_dir_files(fcinfo, XLOGDIR, LS_DIR_HISTORIC); } /* @@ -651,7 +669,8 @@ pg_ls_tmpdir(FunctionCallInfo fcinfo, Oid tblspc) tblspc))); TempTablespacePath(path, tblspc); - return pg_ls_dir_files(fcinfo, path, true); + return pg_ls_dir_files(fcinfo, path, + LS_DIR_HISTORIC | LS_DIR_MISSING_OK); } /* @@ -680,5 +699,33 @@ pg_ls_tmpdir_1arg(PG_FUNCTION_ARGS) Datum pg_ls_archive_statusdir(PG_FUNCTION_ARGS) { - return pg_ls_dir_files(fcinfo, XLOGDIR "/archive_status", true); + return pg_ls_dir_files(fcinfo, XLOGDIR "/archive_status", + LS_DIR_HISTORIC | LS_DIR_MISSING_OK); +} + +/* + * Function to return the list of files and metadata in an arbitrary directory. + */ +Datum +pg_ls_dir_metadata(PG_FUNCTION_ARGS) +{ + char *dirname = convert_and_check_filename(PG_GETARG_TEXT_PP(0)); + + return pg_ls_dir_files(fcinfo, dirname, + LS_DIR_METADATA | LS_DIR_SKIP_SPECIAL | LS_DIR_ISDIR); +} + +/* + * Function to return the list of files and metadata in an arbitrary directory. + * note: this wrapper is necessary to pass the sanity check in opr_sanity, + * which checks that all built-in functions that share the implementing C + * function take the same number of arguments. + */ +Datum +pg_ls_dir_metadata_1arg(PG_FUNCTION_ARGS) +{ + char *dirname = convert_and_check_filename(PG_GETARG_TEXT_PP(0)); + + return pg_ls_dir_files(fcinfo, dirname, + LS_DIR_METADATA | LS_DIR_SKIP_SPECIAL | LS_DIR_ISDIR); } diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 4bce3ad8de..9f46cba5ed 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -10904,6 +10904,18 @@ proallargtypes => '{oid,text,int8,timestamptz}', proargmodes => '{i,o,o,o}', proargnames => '{tablespace,name,size,modification}', prosrc => 'pg_ls_tmpdir_1arg' }, +{ oid => '5032', descr => 'list directory with metadata', + proname => 'pg_ls_dir_metadata', procost => '10', prorows => '20', proretset => 't', + provolatile => 'v', prorettype => 'record', proargtypes => 'text bool bool', + proallargtypes => '{text,bool,bool,text,int8,timestamptz,bool}', proargmodes => '{i,i,i,o,o,o,o}', + proargnames => '{dirname,missing_ok,include_dot_dirs,name,size,modification,isdir}', + prosrc => 'pg_ls_dir_metadata' }, +{ oid => '5033', descr => 'list directory with metadata', + proname => 'pg_ls_dir_metadata', procost => '10', prorows => '20', proretset => 't', + provolatile => 'v', prorettype => 'record', proargtypes => 'text', + proallargtypes => '{text,text,int8,timestamptz,bool}', proargmodes => '{i,o,o,o,o}', + proargnames => '{dirname,name,size,modification,isdir}', + prosrc => 'pg_ls_dir_metadata_1arg' }, # hash partitioning constraint function { oid => '5028', descr => 'hash partition CHECK constraint', diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out index 2e87c548eb..7930909f02 100644 --- a/src/test/regress/expected/misc_functions.out +++ b/src/test/regress/expected/misc_functions.out @@ -219,6 +219,30 @@ select pg_ls_dir('does not exist', true, false); -- ok with missingok=true select pg_ls_dir('does not exist'); -- fails with missingok=false ERROR: could not open directory "does not exist": No such file or directory +-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet +-- The name='' condition is never true, so the function runs to completion but returns zero rows. +select * from pg_ls_tmpdir() where name='Does not exist'; + name | size | modification +------+------+-------------- +(0 rows) + +select name, isdir from pg_ls_dir_metadata('.') where name='.'; + name | isdir +------+------- + . | t +(1 row) + +select name, isdir from pg_ls_dir_metadata('.', false, false) where name='.'; -- include_dot_dirs=false + name | isdir +------+------- +(0 rows) + +-- Check that expected columns are present +select * from pg_ls_dir_metadata('.') limit 0; + name | size | modification | isdir +------+------+--------------+------- +(0 rows) + -- -- Test adding a support function to a subject function -- diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source index a5f61a35dc..0b9cfe615e 100644 --- a/src/test/regress/input/tablespace.source +++ b/src/test/regress/input/tablespace.source @@ -11,6 +11,11 @@ DROP TABLESPACE regress_tblspacewith; -- create a tablespace we can use CREATE TABLESPACE regress_tblspace LOCATION '@testtablespace@'; +-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet +-- The name='' condition is never true, so the function runs to completion but returns zero rows. +-- The query is written to ERROR if the tablespace doesn't exist, rather than silently failing to call pg_ls_tmpdir() +SELECT c.* FROM (SELECT oid FROM pg_tablespace b WHERE b.spcname='regress_tblspace' UNION SELECT 0 ORDER BY 1 DESC LIMIT 1) AS b , pg_ls_tmpdir(oid) AS c WHERE c.name='Does not exist'; + -- try setting and resetting some properties for the new tablespace ALTER TABLESPACE regress_tblspace SET (random_page_cost = 1.0, seq_page_cost = 1.1); ALTER TABLESPACE regress_tblspace SET (some_nonexistent_parameter = true); -- fail diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source index 162b591b31..a42714bf40 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -13,6 +13,14 @@ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith'; DROP TABLESPACE regress_tblspacewith; -- create a tablespace we can use CREATE TABLESPACE regress_tblspace LOCATION '@testtablespace@'; +-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet +-- The name='' condition is never true, so the function runs to completion but returns zero rows. +-- The query is written to ERROR if the tablespace doesn't exist, rather than silently failing to call pg_ls_tmpdir() +SELECT c.* FROM (SELECT oid FROM pg_tablespace b WHERE b.spcname='regress_tblspace' UNION SELECT 0 ORDER BY 1 DESC LIMIT 1) AS b , pg_ls_tmpdir(oid) AS c WHERE c.name='Does not exist'; + name | size | modification +------+------+-------------- +(0 rows) + -- try setting and resetting some properties for the new tablespace ALTER TABLESPACE regress_tblspace SET (random_page_cost = 1.0, seq_page_cost = 1.1); ALTER TABLESPACE regress_tblspace SET (some_nonexistent_parameter = true); -- fail diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql index f6857ad177..372345720d 100644 --- a/src/test/regress/sql/misc_functions.sql +++ b/src/test/regress/sql/misc_functions.sql @@ -65,6 +65,17 @@ select * from (select pg_ls_dir('.', false, false) as name) as ls where ls.name= select pg_ls_dir('does not exist', true, false); -- ok with missingok=true select pg_ls_dir('does not exist'); -- fails with missingok=false +-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet +-- The name='' condition is never true, so the function runs to completion but returns zero rows. +select * from pg_ls_tmpdir() where name='Does not exist'; + +select name, isdir from pg_ls_dir_metadata('.') where name='.'; + +select name, isdir from pg_ls_dir_metadata('.', false, false) where name='.'; -- include_dot_dirs=false + +-- Check that expected columns are present +select * from pg_ls_dir_metadata('.') limit 0; + -- -- Test adding a support function to a subject function -- -- 2.17.0 --4LFBTxd4L5NLO6ly Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v17-0005-pg_ls_tmpdir-to-show-directories-and-isdir-argum.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* Fix inappropriate uses of atol() @ 2024-08-03 11:04 Peter Eisentraut <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Peter Eisentraut @ 2024-08-03 11:04 UTC (permalink / raw) To: pgsql-hackers 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 ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-08-03 11:04 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-03-10 03:40 [PATCH v17 04/10] Add pg_ls_dir_metadata to list a dir with file metadata.. Justin Pryzby <[email protected]> 2024-08-03 11:04 Fix inappropriate uses of atol() Peter Eisentraut <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox