Trying to clean up the code a bit while we're working on these files
for the reentrant scanner/pure parser patches. This cleanup only
touches the code sections after the second '%%' in each file, via a
manually-supervised and locally hacked up pgindent.
This commit adds one field to PGconn for the database service name (if
any), with PQservice() as routine to retrieve it. Like the other
routines of this area, NULL is returned as result if the connection is
NULL.
A follow-up patch will make use of this feature to be able to display
the service name in the psql prompt.
Author: Michael Banck
Reviewed-by: Greg Sabino Mullane
Discusion: https://postgr.es/m/6723c612.050a0220.1567f4.b94a@mx.google.com
Redefine our exact width types with standard C99 types and macros,
including int64_t, INT64_MAX, INT64_C(), PRId64 etc. We were already
using <stdint.h> types in a few places.
One complication is that Windows' <inttypes.h> uses format strings like
"%I64d", "%I32", "%I" for PRI*64, PRI*32, PTR*PTR, instead of mapping to
other standardized format strings like "%lld" etc as seen on other known
systems. Teach our snprintf.c to understand them.
This removes a lot of configure clutter, and should also allow 64-bit
numbers and other standard types to be used in localized messages
without casting.
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/ME3P282MB3166F9D1F71F787929C0C7E7B6312%40ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM
The loops over cursor argument variables neglected to ever advance
"prevvar". The code would accidentally do the right thing anyway
when removing the first or second list entry, but if it had to
remove the third or later entry then it would also remove all
entries between there and the first entry. AFAICS this would
only matter for cursors that reference out-of-scope variables,
which is a weird Informix compatibility hack; between that and
the lack of impact for short lists, it's not so surprising that
nobody has complained. Nonetheless it's a pretty obvious bug.
It would have been more obvious if these loops used a more standard
coding style for chasing the linked lists --- this business with the
"prev" pointer sometimes pointing at the current list entry is
confusing and overcomplicated. So rather than just add a minimal
band-aid, I chose to rewrite the loops in the same style we use
elsewhere, where the "prev" pointer is NULL until we are dealing with
a non-first entry and we save the "next" pointer at the top of the
loop. (Two of the four loops touched here are not actually buggy,
but it seems better to make them all look alike.)
Coverity discovered this problem, but not until 2b41de4a5 added code
to free no-longer-needed arguments structs. With that, the incorrect
link updates are possibly touching freed memory, and it complained
about that. Nonetheless the list corruption hazard is ancient, so
back-patch to all supported branches.
The C standard says that sizeof(bool) is implementation-defined, but we
know of no current systems where it is not 1. The last known systems
seem to have been Apple macOS/PowerPC 10.5 and Microsoft Visual C++ 4,
both long defunct.
PostgreSQL has always required sizeof(bool) == 1 for the definition of
bool that it used, but previously it would define its own type if the
system-provided bool had a different size. That was liable to cause
memory layout problems when interacting with system and third-party
libraries on (by now hypothetical) computers with wider _Bool, and now
C23 has introduced a new problem by making bool a built-in datatype
(like C++), so the fallback code doesn't even compile. We could
probably work around that, but then we'd be writing new untested code
for a computer that doesn't exist.
Instead, delete the unreachable and C23-uncompilable fallback code, and
let existing static assertions fail if the system-provided bool is too
wide. If we ever get a problem report from a real system, then it will
be time to figure out what to do about it in a way that also works on
modern compilers.
Note on C++: Previously we avoided including <stdbool.h> or trying to
define a new bool type in headers that might be included by C++ code.
These days we might as well just include <stdbool.h> unconditionally:
it should be visible to C++11 but do nothing, just as in C23. We
already include <stdint.h> without C++ guards in c.h, and that falls
under the same C99-compatibility section of the C++11 standard as
<stdbool.h>, so let's remove the guards here too.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/3198438.1731895163%40sss.pgh.pa.us
Avoid leaking the prior value when updating the "connection"
state variable.
Ditto for ECPGstruct_sizeof. (It seems like this one ought to
be statement-local, but testing says it isn't, and I didn't
feel like diving deeper.)
The actual_type[] entries are statement-local, though, so
no need to mm_strdup() strings stored in them.
Likewise, sqlda variables are statement-local, so we can
loc_alloc them.
Also clean up sloppiness around management of the argsinsert and
argsresult lists.
progname changes are strictly to prevent valgrind from complaining
about leaked allocations.
With this, valgrind reports zero leakage in the ecpg preprocessor
for all of our ecpg regression test cases.
Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
This didn't work earlier in the patch series (I think some of
the strings were ending up in data-type-related structures),
but apparently we're now clean enough for it. This considerably
reduces process-lifespan memory leakage.
Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
ECPGfree_type() and related functions were quite incomplete
about removing subsidiary data structures. Possibly this is
because ecpg wasn't careful to make sure said data structures
always had their own storage. Previous patches in this series
cleaned up a lot of that, and I had to add a couple more
mm_strdup's here.
Also, ecpg.trailer tended to overwrite struct_member_list[struct_level]
without bothering to free up its previous contents, thus potentially
leaking a lot of struct-member-related storage. Add
ECPGfree_struct_member() calls at appropriate points.
Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
This reverts commit 2cf91ccb73.
When using the old msvcrt.dll, MinGW would supply its own dummy version
of _configthreadlocale() that just returns -1 if you try to use it. For
a time we tolerated that to shut the build farm up. We would fall back
to code that was enough for the tests to pass, but it would surely have
risked crashing a real multithreaded program.
We don't need that kludge anymore, because we can count on ucrt. We
expect the real _configthreadlocale() to be present, and the ECPG tests
will now fail if it isn't. The workaround was dead code and it's time
to revert it.
(A later patch still under review proposes to remove this use of
_configthreadlocale() completely but we're unwinding this code in
steps.)
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/d9e7731c-ca1b-477c-9298-fa51e135574a%40eisentraut.org
All modern Windows systems have _configthreadlocale(). It was first
introduced in msvcr80.dll from Visual Studio 2005. Historically, MinGW
was stuck on even older msvcrt.dll, but added its own dummy
implementation of the function when using msvcrt.dll years ago anyway,
effectively rendering the configure test useless. In practice we don't
encounter the dummy anymore because modern MinGW uses ucrt.
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech
The error message showing up when parameters or keywords include too
many whitespaces was "trailing data found", which was confusing because
there was no hint about what was actually wrong.
Issue introduced in 430ce189fc, hence there is no need for a
backpatch.
Author: Yushi Ogiwara
Reviewed-by: Fujii Masao, Tom Lane, Bruce Momjian
Discussion: https://postgr.es/m/645bd22a53c4da8a1bc7e1e52d9d3b52@oss.nttdata.com
This commit changes libpq so that errors reported by the backend during
the protocol negotiation for SSL and GSS are discarded by the client, as
these may include bytes that could be consumed by the client and write
arbitrary bytes to a client's terminal.
A failure with the SSL negotiation now leads to an error immediately
reported, without a retry on any other methods allowed, like a fallback
to a plaintext connection.
A failure with GSS discards the error message received, and we allow a
fallback as it may be possible that the error is caused by a connection
attempt with a pre-11 server, GSS encryption having been introduced in
v12. This was a problem only with v17 and newer versions; older
versions discard the error message already in this case, assuming a
failure caused by a lack of support for GSS encryption.
Author: Jacob Champion
Reviewed-by: Peter Eisentraut, Heikki Linnakangas, Michael Paquier
Security: CVE-2024-10977
Backpatch-through: 12
SASL frontend mechanisms are implemented with pg_fe_sasl_mech and
not the _be_ variant which is the backend implementation. Spotted
while reading adjacent code.
It was possible for the code to read out-of-bound data from the
"day_tab" table with some crafted input data. Let's treat these as
invalid input as the month number is incorrect.
A test is added to test this case with a check on the errno returned by
the decoding routine. A test close to the new one added in this commit
was testing for a failure, but did not look at the errno generated, so
let's use this commit to also change it, adding a check on the errno
returned by DecodeDateTime().
Like the other test scripts, dt_test should likely be expanded to
include more checks based on the errnos generated in these code paths.
This is left as future work.
This issue exists since 2e6f97560a, so backpatch all the way down.
Reported-by: Pavel Nekrasov
Author: Bruce Momjian, Pavel Nekrasov
Discussion: https://postgr.es/m/18614-6bbe00117352309e@postgresql.org
Backpatch-through: 12
Previously, ecpg_log() always called ECPGget_sqlca() to retrieve sqlca,
even though it was only needed for debug logging. This commit updates
ecpg_log() to call ECPGget_sqlca() only when debug logging is enabled.
Author: Yuto Sasaki
Reviewed-by: Alvaro Herrera, Tom Lane, Fujii Masao
Discussion: https://postgr.es/m/TY2PR01MB3628A85689649BABC9A1C6C3C1782@TY2PR01MB3628.jpnprd01.prod.outlook.com
Don't get confused by an unmatched right brace in the input.
(Previously, this led to discarding information about file-level
variables and then possibly crashing.)
Detect, rather than crash on, an attempt to index into a non-array
variable.
As before, in the absence of field complaints I'm not too
excited about back-patching these.
Per valgrind testing by Alexander Lakhin.
Discussion: https://postgr.es/m/a239aec2-6c79-5fc9-9272-cea41158a360@gmail.com
Avoid null-pointer crash when considering a cursor declaration
that's outside any C function (a case which is useless anyway).
Ensure a cursor for a prepared statement is marked as initially
not open. At worst, if we chanced to get not-already-zeroed memory
from malloc(), this oversight would result in failing to issue a
"cursor "foo" has been declared but not opened" warning that would
have been appropriate.
Avoid running off the end of the buffer when there are mismatched
square brackets following a variable name. This could lead to
SIGSEGV after reaching the end of memory.
Given the lack of field complaints, none of these seem to be worth
back-patching, but let's clean them up in HEAD.
Per valgrind testing by Alexander Lakhin.
Discussion: https://postgr.es/m/5f5bcecd-d7ec-b8c0-6c92-d1a7c6e0f639@gmail.com
Put the rule type at the start not the end, and put spaces
between the constitutent token names instead of smashing them
into an illegible mess. This has no functional impact but
I think it makes the rules a great deal more readable.
Discussion: https://postgr.es/m/1185216.1724001216@sss.pgh.pa.us
parse.pl contains several constant tables that describe tweaks
to be made to the backend grammar. In the same spirit as
00b0e7204, add cross-checks that each table entry is used at
least once (or exactly once if that's appropriate). This should
help catch cases where adjustments to the backend grammar cause
a table entry not to match as expected.
Per suggestion from Michael Paquier.
Discussion: https://postgr.es/m/ZsLVbjsc5x5Saesg@paquier.xyz
Careless string hacking caused parse.pl to transform gram.y's
declaration
%nonassoc IDENT PARTITION RANGE ROWS ...
into
%nonassoc IDENT
%nonassoc CSTRING PARTITION RANGE ROWS ...
It turns out that this has no semantic impact, because the
generated preproc.c is exactly the same either way (if you
inject a blank line to keep line numbers the same).
Nonetheless, given the great emphasis that the commentary in
gram.y places on keeping those other keywords at the same
precedence level as IDENT, this seems like foolishly risking ecpg
behaving differently from the core parser. Adjust the code so
that CSTRING is added to the precedence line without breaking it
into two lines.
Discussion: https://postgr.es/m/2157151.1713540065@sss.pgh.pa.us
Invent a notion of "local" storage that will automatically be
reclaimed at the end of each statement. Use this for location
strings as well as other visibly short-lived data within the parser.
Also, make cat_str and make_str return local storage and not free
their inputs, which allows dispensing with a whole lot of retail
mm_strdup calls. We do have to add some new ones in places where
a local-lifetime string needs to be added to a longer-lived data
structure, but on balance there are a lot less mm_strdup calls than
before.
In hopes of flushing out places where changes were necessary,
I changed YYLTYPE from "char *" to "const char *", which forced
const-ification of various function arguments that probably
should've been like that all along.
This still leaks somewhat more memory than v17, but that will be
cleaned up in future commits.
Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
mm_alloc and mm_strdup were in type.c, which seems a completely
random choice. No doubt the original author thought two small
functions didn't deserve their own file. But I'm about to add
some more memory-management stuff beside them, so let's put them
in a less surprising place. This seems like a better home for
mmerror, mmfatal, and the cat_str/make_str family, too.
Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
Most productions in the preprocessor grammar construct strings
representing SQL or C statements or fragments thereof. Instead
of returning these as <str> results of the productions, return
them as "location" values, taking advantage of Bison's flexibility
about what a location is. We aren't really giving up anything
thereby, since ecpg's error reports have always just given line
numbers, and that's tracked separately. The advantage of this
is that a single instance of the YYLLOC_DEFAULT macro can
perform all the work needed by the vast majority of productions,
including all the ones made automatically by parse.pl. This
avoids having large numbers of effectively-identical productions,
which tickles an optimization inefficiency in recent versions of
clang. (This patch reduces the compilation time for preproc.o
by more than 100-fold with clang 16, and is visibly helpful with
gcc too.) The compiled parser is noticeably smaller as well.
A disadvantage of this approach is that YYLLOC_DEFAULT is applied
before running the production's semantic action (if any). This
means it cannot use the method favored by cat_str() of free'ing
all the input strings; if the action needs to look at the input
strings, it'd be looking at dangling storage. As this stands,
therefore, it leaks memory like a sieve. This is already a big
patch though, and fixing the memory management seems like a
separable problem, so let's leave that for the next step.
(This does remove some free() calls that I'd have had to touch
anyway, in the expectation that the next step will manage
memory reclamation quite differently.)
Most of the changes here are mindless substitution of "@N" for
"$N" in grammar rules; see the changes to README.parser for
an explanation.
Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
Remove a lot of cruft, clean up and document what's left.
This produces the same preproc.y output as before, except for
fewer blank lines. (It's not like we're making any attempt to
match the layout of gram.y, so I removed the one bit of logic
that seemed to have that in mind.)
Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
As noted in the previous commit, check_rules.pl is now entirely
redundant with checks made by parse.pl, or would be if it weren't
for the places where it's wrong. It's a waste of build cycles
and maintenance effort, so remove it.
Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
README.parser is the user's manual, such as it is, for parse.pl.
It's rather poorly written if you ask me; so try to improve it.
(More could be written here, but this at least covers the same
info in a more organized fashion.)
Also, the single solitary line of usage info in parse.pl itself
was a lie. Replace.
Add some error checks that the ecpg.addons entries meet the syntax
rules set forth in README.parser. One of them didn't, but
accidentally worked anyway because the logic in include_addon is
such that 'block' is the default behavior.
Also add a cross-check that each ecpg.addons entry is matched exactly
once in the backend grammar. This exposed that there are two dead
entries there --- they are dead because the %replace_types table in
parse.pl causes their nonterminals to be ignored altogether.
Removing them doesn't change the generated preproc.y file.
(This implies that check_rules.pl is completely worthless and should
be nuked: it adds build cycles and maintenance effort while failing
to reliably accomplish its one job of detecting dead rules. I'll
do that separately.)
Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
PostgreSQL has for a long time mixed two BIO implementations, which can
lead to subtle bugs and inconsistencies. This cleans up our BIO by just
just setting up the methods we need. This patch does not introduce any
functionality changes.
The following methods are no longer defined due to not being needed:
- gets: Not used by libssl
- puts: Not used by libssl
- create: Sets up state not used by libpq
- destroy: Not used since libpq use BIO_NOCLOSE, if it was used it close
the socket from underneath libpq
- callback_ctrl: Not implemented by sockets
The following methods are defined for our BIO:
- read: Used for reading arbitrary length data from the BIO. No change
in functionality from the previous implementation.
- write: Used for writing arbitrary length data to the BIO. No change
in functionality from the previous implementation.
- ctrl: Used for processing ctrl messages in the BIO (similar to ioctl).
The only ctrl message which matters is BIO_CTRL_FLUSH used for
writing out buffered data (or signal EOF and that no more data
will be written). BIO_CTRL_FLUSH is mandatory to implement and
is implemented as a no-op since there is no intermediate buffer
to flush.
BIO_CTRL_EOF is the out-of-band method for signalling EOF to
read_ex based BIO's. Our BIO is not read_ex based but someone
could accidentally call BIO_CTRL_EOF on us so implement mainly
for completeness sake.
As the implementation is no longer related to BIO_s_socket or calling
SSL_set_fd, methods have been renamed to reference the PGconn and Port
types instead.
This also reverts back to using BIO_set_data, with our fallback, as a small
optimization as BIO_set_app_data require the ex_data mechanism in OpenSSL.
Author: David Benjamin <davidben@google.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CAF8qwaCZ97AZWXtg_y359SpOHe+HdJ+p0poLCpJYSUxL-8Eo8A@mail.gmail.com
Integer values applied a parsing rule through pqParseIntParam() that
made URIs like this one working, even if these include spaces around
values:
"postgresql://localhost:5432/postgres?keepalives=1 &keepalives_idle=1 "
This commit changes the parsing so as spaces before and after parameters
and values are discarded, offering more consistency with the parsing
that already applied to libpq for integer values in URIs.
Note that %20 can be used in a URI for a space character. ECPGconnect()
has been discarded leading and trailing spaces around parameters and
values that for a long time, as well. Like f22e84df1d, this is done
as a HEAD-only change.
Reviewed-by: Yuto Sasaki
Discussion: https://postgr.es/m/Zv3oWOfcrHTph7JK@paquier.xyz
The preprocessor really should not have done this to begin with.
The space after '&' was masked by ECPGconnect's skipping spaces
before option keywords, and the space before by dint of libpq
being (mostly) insensitive to trailing space in option values.
We fixed the one known problem with that in 920d51979. Hence
this patch is mostly cosmetic, and we'll just change it in HEAD.
Discussion: https://postgr.es/m/TY2PR01MB36286A7B97B9A15793335D18C1772@TY2PR01MB3628.jpnprd01.prod.outlook.com
Use pqParseIntParam (nee parse_int_param) instead of using strtol
directly. This allows trailing whitespace, which the previous coding
didn't, and makes the spelling of the error message consistent with
other similar cases.
This seems to be an oversight in commit e7a221797, which introduced
parse_int_param. That fixed places that were using atoi(), but missed
this place which was randomly using strtol() instead.
Ordinarily I'd consider this minor cleanup not worth back-patching.
However, it seems that ecpg assumes it can add trailing whitespace
to URL parameters, so that use of the keepalives option fails in
that context. Perhaps that's worth improving as a separate matter.
In the meantime, back-patch this to all supported branches.
Yuto Sasaki (some further cleanup by me)
Discussion: https://postgr.es/m/TY2PR01MB36286A7B97B9A15793335D18C1772@TY2PR01MB3628.jpnprd01.prod.outlook.com
Instead of XXX_IN_XLOCALE_H for several features XXX, let's just
include <xlocale.h> if HAVE_XLOCALE_H. The reason for the extra
complication was apparently that some old glibc systems also had an
<xlocale.h>, and you weren't supposed to include it directly, but it's
gone now (as far as I can tell it was harmless to do so anyway).
Author: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech
The existing function PQprotocolVersion() does not include the minor
version of the protocol. In preparation for pending work that will
bump that number for the first time, add a new function to provide it
to clients that may care, using the (major * 10000 + minor)
convention already used by PQserverVersion().
Jacob Champion based on earlier work by Jelte Fennema-Nio
Discussion: http://postgr.es/m/CAOYmi+mM8+6Swt1k7XsLcichJv8xdhPnuNv7-02zJWsezuDL+g@mail.gmail.com
Looking up a nonexistent user ID would lead to a null pointer
dereference. That's unlikely to happen here, but perhaps
not impossible.
Thinko in commit 4d5111b3f, noticed by Coverity.
Since commit 2549f0661, we reject an identifier immediately following
a numeric literal (without separating whitespace), because that risks
ambiguity with hex/octal/binary integers. However, that patch used
token patterns like "{integer}{ident_start}", which is problematic
because {ident_start} matches only a single byte. If the first
character after the integer is a multibyte character, this ends up
with flex reporting an error message that includes a partial multibyte
character. That can cause assorted bad-encoding problems downstream,
both in the report to the client and in the postmaster log file.
To fix, use {identifier} not {ident_start} in the "junk" token
patterns, so that they will match complete multibyte characters.
This seems generally better user experience quite aside from the
encoding problem: for "123abc" the error message will now say that
the error appeared at or near "123abc" instead of "123a".
While at it, add some commentary about why these patterns exist
and how they work.
Report and patch by Karina Litskevich; review by Pavel Borisov.
Back-patch to v15 where the problem came in.
Discussion: https://postgr.es/m/CACiT8iZ_diop=0zJ7zuY3BXegJpkKK1Av-PU7xh0EDYHsa5+=g@mail.gmail.com
This fixes defects with installcheck for TAP tests that expect the
module injection_points to exist in an installation, but the contents of
src/test/modules are not installed by default with installcheck. This
would cause, for example, failures under installcheck-world for a build
with injection points enabled, when the contents of src/test/modules/
are not installed.
The availability of the module can be done with a scan of
pg_available_extension. This has been introduced in 2cdcae9da6, and
it is refactored here as a new routine in Cluster.pm.
Tests are changed in different ways depending on what they need:
- The libpq TAP test sets up a node even without injection points, so it
is enough to check that CREATE EXTENSION can be used. There is no need
for the variable enable_injection_points.
- In test_misc, 006_signal_autovacuum requires a runtime check.
- 041_checkpoint_at_promote in recovery tests and 005_timeouts in
test_misc are updated to use the routine introduced in Cluster.pm.
- test_slru's 001_multixact, injection_points's 001_stats and
modules/gin/ do not require a check as these modules disable
installcheck entirely.
Discussion: https://postgr.es/m/ZtesYQ-WupeAK7xK@paquier.xyz
The typos in 005_negotiate_encryption.pl and pg_combinebackup.c
shall be backported to v17 where they were introduced.
Backpatch-through: v17
Discussion: https://postgr.es/m/Ztaj7BkN4658OMxF@paquier.xyz
Remove src/port/user.c, call getpwuid_r() directly. This reduces some
complexity and allows better control of the error behavior. For
example, the old code would in some circumstances silently truncate
the result string, or produce error message strings that the caller
wouldn't use.
src/port/user.c used to be called src/port/thread.c and contained
various portability complications to support thread-safety. These are
all obsolete, and all but the user-lookup functions have already been
removed. This patch completes this by also removing the user-lookup
functions.
Also convert src/backend/libpq/auth.c to use getpwuid_r() for
thread-safety.
Originally, I tried to be overly correct by using
sysconf(_SC_GETPW_R_SIZE_MAX) to get the buffer size for getpwuid_r(),
but that doesn't work on FreeBSD. All the OS where I could find the
source code internally use 1024 as the suggested buffer size, so I
just ended up hardcoding that. The previous code used BUFSIZ, which
is an unrelated constant from stdio.h, so its use seemed
inappropriate.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/5f293da9-ceb4-4937-8e52-82c25db8e4d3%40eisentraut.org
As per the policy established in commit 6991e774e, invent macros
that can be tested at compile time to detect presence of new libpq
features. This should make calling code more readable and less
error-prone than checking the libpq version would be (especially
since we don't expose that at compile time; the server version is
an unreliable substitute).
Discussion: https://postgr.es/m/2042418.1724346970@sss.pgh.pa.us
Use gmtime_r() and localtime_r() instead of gmtime() and localtime(),
for thread-safety.
There are a few affected calls in libpq and ecpg's libpgtypes, which
are probably effectively bugs, because those libraries already claim
to be thread-safe.
There is one affected call in the backend. Most of the backend
otherwise uses the custom functions pg_gmtime() and pg_localtime(),
which are implemented differently.
While we're here, change the call in the backend to gmtime*() instead
of localtime*(), since for that use time zone behavior is irrelevant,
and this side-steps any questions about when time zones are
initialized by localtime_r() vs localtime().
Portability: gmtime_r() and localtime_r() are in POSIX but are not
available on Windows. Windows has functions gmtime_s() and
localtime_s() that can fulfill the same purpose, so we add some small
wrappers around them. (Note that these *_s() functions are also
different from the *_s() functions in the bounds-checking extension of
C11. We are not using those here.)
On MinGW, you can get the POSIX-style *_r() functions by defining
_POSIX_C_SOURCE appropriately before including <time.h>. This leads
to a conflict at least in plpython because apparently _POSIX_C_SOURCE
gets defined in some header there, and then our replacement
definitions conflict with the system definitions. To avoid that sort
of thing, we now always define _POSIX_C_SOURCE on MinGW and use the
POSIX-style functions here.
Reviewed-by: Stepan Neretin <sncfmgg@gmail.com>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/eba1dc75-298e-4c46-8869-48ba8aad7d70@eisentraut.org
Not all messages that libpq received from the server would be sent
through our message tracing logic. This commit tries to fix that by
introducing a new function pqParseDone which make it harder to forget
about doing so.
The messages that we now newly send through our tracing logic are:
- CopyData (received by COPY TO STDOUT)
- Authentication requests
- NegotiateProtocolVersion
- Some ErrorResponse messages during connection startup
- ReadyForQuery when received after a FunctionCall message
Author: Jelte Fennema-Nio <postgres@jeltef.nl>
Discussion: https://postgr.es/m/CAGECzQSoPHtZ4xe0raJ6FYSEiPPS+YWXBhOGo+Y1YecLgknF3g@mail.gmail.com
libpq checks the permissions of the password file before opening it.
The way this is done in two separate operations, a static analyzer
would flag as a time-of-check-time-of-use violation. In practice, you
can't do anything with that, but it still seems better style to fix
it.
To fix it, open the file first and then check the permissions on the
opened file handle.
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Discussion: https://www.postgresql.org/message-id/flat/a3356054-14ae-4e7a-acc6-249d19dac20b%40eisentraut.org
This commit attempts to update a few places, such as the money,
numeric, and timestamp types, to no longer rely on signed integer
wrapping for correctness. This is intended to move us closer
towards removing -fwrapv, which may enable some compiler
optimizations. However, there is presently no plan to actually
remove that compiler option in the near future.
Besides using some of the existing overflow-aware routines in
int.h, this commit introduces and makes use of some new ones.
Specifically, it adds functions that accept a signed integer and
return its absolute value as an unsigned integer with the same
width (e.g., pg_abs_s64()). It also adds functions that accept an
unsigned integer, store the result of negating that integer in a
signed integer with the same width, and return whether the negation
overflowed (e.g., pg_neg_u64_overflow()).
Finally, this commit adds a couple of tests for timestamps near
POSTGRES_EPOCH_JDATE.
Author: Joseph Koshakow
Reviewed-by: Tom Lane, Heikki Linnakangas, Jian He
Discussion: https://postgr.es/m/CAAvxfHdBPOyEGS7s%2Bxf4iaW0-cgiq25jpYdWBqQqvLtLe_t6tw%40mail.gmail.com
