summaryrefslogtreecommitdiff
path: root/src/port/snprintf.c
AgeCommit message (Collapse)Author
2025-04-01Fix detection and handling of strchrnul() for macOS 15.4.Tom Lane
As of 15.4, macOS has strchrnul(), but access to it is blocked behind a check for MACOSX_DEPLOYMENT_TARGET >= 15.4. But our does-it-link configure check finds it, so we try to use it, and fail with the present default deployment target (namely 15.0). This accounts for today's buildfarm failures on indri and sifaka. This is the identical problem that we faced some years ago when Apple introduced preadv and pwritev in the same way. We solved that in commit f014b1b9b by using AC_CHECK_DECLS instead of AC_CHECK_FUNCS to check the functions' availability. So do the same now for strchrnul(). Interestingly, we already had a workaround for "the link check doesn't agree with <string.h>" cases with glibc, which we no longer need since only the header declaration is being checked. Testing this revealed that the meson version of this check has never worked, because it failed to use "-Werror=unguarded-availability-new". (Apparently nobody's tried to build with meson on macOS versions that lack preadv/pwritev as standard.) Adjust that while at it. Also, we had never put support for "-Werror=unguarded-availability-new" into v13, but we need that now. Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us> Co-authored-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/385134.1743523038@sss.pgh.pa.us Backpatch-through: 13
2025-03-28Revert "Tidy up locale thread safety in ECPG library."Peter Eisentraut
This reverts commit 8e993bff5326b00ced137c837fce7cd1e0ecae14. It causes various build failures on the buildfarm, to be investigated. Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech
2025-03-28Tidy up locale thread safety in ECPG library.Peter Eisentraut
Remove setlocale() and _configthreadlocal() as fallback strategy on systems that don't have uselocale(), where ECPG tries to control LC_NUMERIC formatting on input and output of floating point numbers. It was probably broken on some systems (NetBSD), and the code was also quite messy and complicated, with obsolete configure tests (Windows). It was also arguably broken, or at least had unstated environmental requirements, if pgtypeslib code was called directly. Instead, introduce PG_C_LOCALE to refer to the "C" locale as a locale_t value. It maps to the special constant LC_C_LOCALE when defined by libc (macOS, NetBSD), or otherwise uses a process-lifetime locale_t that is allocated on first use, just as ECPG previously did itself. The new replacement might be more widely useful. Then change the float parsing and printing code to pass that to _l() functions where appropriate. Unfortunately the portability of those functions is a bit complicated. First, many obvious and useful _l() functions are missing from POSIX, though most standard libraries define some of them anyway. Second, although the thread-safe save/restore technique can be used to replace the missing ones, Windows and NetBSD refused to implement standard uselocale(). They might have a point: "wide scope" uselocale() is hard to combine with other code and error-prone, especially in library code. Luckily they have the _l() functions we want so far anyway. So we have to be prepared for both ways of doing things: 1. In ECPG, use strtod_l() for parsing, and supply a port.h replacement using uselocale() over a limited scope if missing. 2. Inside our own snprintf.c, use three different approaches to format floats. For frontend code, call libc's snprintf_l(), or wrap libc's snprintf() in uselocale() if it's missing. For backend code, snprintf.c can keep assuming that the global locale's LC_NUMERIC is "C" and call libc's snprintf() without change, for now. (It might eventually be possible to call our in-tree Ryū routines to display floats in snprintf.c, given the C-locale-always remit of our in-tree snprintf(), but this patch doesn't risk changing anything that complicated.) Author: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Reviewed-by: Tristan Partin <tristan@partin.io> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech
2025-01-01Update copyright for 2025Bruce Momjian
Backpatch-through: 13
2024-12-04Use <stdint.h> and <inttypes.h> for c.h integers.Thomas Munro
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
2024-01-03Update copyright for 2024Bruce Momjian
Reported-by: Michael Paquier Discussion: https://postgr.es/m/ZZKTDPxBBMt3C0J9@paquier.xyz Backpatch-through: 12
2023-01-02Update copyright for 2023Bruce Momjian
Backpatch-through: 11
2022-10-16Use libc's snprintf, not sprintf, for special cases in snprintf.c.Tom Lane
snprintf.c has always fallen back on libc's *printf implementation when printing pointers (%p) and floats. When this code originated, we were still supporting some platforms that lacked native snprintf, so we used sprintf for that. That's not actually unsafe in our usage, but nonetheless builds on macOS are starting to complain about sprintf being unconditionally deprecated; and I wouldn't be surprised if other platforms follow suit. There seems little reason to believe that any platform supporting C99 wouldn't have standards-compliant snprintf, so let's just use that instead to suppress such warnings. Back-patch to v12, which is where we started to require C99. It's also where we started to use our snprintf.c everywhere, so this wouldn't be enough to suppress the warning in older branches anyway --- that is, in older branches these aren't necessarily all our usages of libc's sprintf. It is enough in v12+ because any deprecation annotation attached to libc's sprintf won't apply to pg_sprintf. (Whether all our usages of pg_sprintf are adequately safe is not a matter I intend to address here, but perhaps it could do with some review.) Per report from Andres Freund and local testing. Discussion: https://postgr.es/m/20221015211955.q4cwbsfkyk3c4ty3@awork3.anarazel.de
2022-07-16Replace many MemSet calls with struct initializationPeter Eisentraut
This replaces all MemSet() calls with struct initialization where that is easily and obviously possible. (For example, some cases have to worry about padding bits, so I left those.) (The same could be done with appropriate memset() calls, but this patch is part of an effort to phase out MemSet(), so it doesn't touch memset() calls.) Reviewed-by: Ranier Vilela <ranier.vf@gmail.com> Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/9847b13c-b785-f4e2-75c3-12ec77a3b05c@enterprisedb.com
2022-01-07Update copyright for 2022Bruce Momjian
Backpatch-through: 10
2021-10-28Speed up printing of integers in snprintf.c.Tom Lane
Since the only possible divisors are 8, 10, and 16, it doesn't cost much code space to replace the division loop with three copies using constant divisors. On most machines, division by a constant can be done a lot more cheaply than division by an arbitrary value. A microbenchmark testing just snprintf("foo %d") with a 9-digit value showed about a 2X speedup for me (tgl). Most of Postgres isn't too dependent on the speed of snprintf, so that the effect in real-world cases is barely measurable. Still, a cycle saved is a cycle earned. Arjan van de Ven Discussion: https://postgr.es/m/40a4b32a-b841-4667-11b2-a0baedb12714@linux.intel.com Discussion: https://postgr.es/m/6e51c644-1b6d-956e-ac24-2d1b0541d532@linux.intel.com
2021-07-24Make printf("%s", NULL) print "(null)" instead of crashing.Tom Lane
We previously took a hard-line attitude that callers should never print a null string pointer, and doing so is worthy of an assertion failure or crash. However, we've long since flushed out any easy-to-find bugs of that nature. What remains is a lot of code that perhaps could fail that way in hard-to-reach corner cases. For example, in something as simple as ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("constraint \"%s\" for table \"%s\" does not exist", conname, get_rel_name(relid)))); one must wonder whether it's completely guaranteed that get_rel_name cannot return NULL in this context. If such a situation did occur, the existing policy converts what might be a pretty minor bug into a server crash condition. This is not good for robustness. Hence, let's follow the lead of glibc and print "(null)" instead of failing. We should, of course, still consider it a bug if that behavior is reachable in ordinary use; but crashing seems less desirable than not crashing. This fix works across-the-board in v12 and up, where we always use src/port/snprintf.c. Before that, on most platforms we're at the mercy of the local libc, but it appears that Solaris 10 is the only supported platform where we'd still get a crash. Most other platforms such as *BSD, macOS, and Solaris 11 have adopted glibc's behavior at some point. (AIX and HPUX just print "" not "(null)", but that's close enough.) I've not checked what Windows' native printf would do, but it doesn't matter because we've long used snprintf.c on that platform. In v12 and up, also const-ify related code so that we're not casting away const on the constant string. This is just neatnik-ism, since next to no compilers will warn about that. Discussion: https://postgr.es/m/17098-b960f3616c861f83@postgresql.org
2021-01-02Update copyright for 2021Bruce Momjian
Backpatch-through: 9.5
2020-05-13Dial back -Wimplicit-fallthrough to level 3Alvaro Herrera
The additional pain from level 4 is excessive for the gain. Also revert all the source annotation changes to their original wordings, to avoid back-patching pain. Discussion: https://postgr.es/m/31166.1589378554@sss.pgh.pa.us
2020-05-12Add -Wimplicit-fallthrough to CFLAGS and CXXFLAGSAlvaro Herrera
Use it at level 4, a bit more restrictive than the default level, and tweak our commanding comments to FALLTHROUGH. (However, leave zic.c alone, since it's external code; to avoid the warnings that would appear there, change CFLAGS for that file in the Makefile.) Author: Julien Rouhaud <rjuju123@gmail.com> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20200412081825.qyo5vwwco3fv4gdo@nol Discussion: https://postgr.es/m/flat/E1fDenm-0000C8-IJ@gemulon.postgresql.org
2020-01-01Update copyrights for 2020Bruce Momjian
Backpatch-through: update all files in master, backpatch legal files through 9.4
2019-11-24Remove a couple of unnecessary if-tests.Tom Lane
Commit abd9ca377 replaced a couple of while-loops in fmtfloat() with calls to dopr_outchmulti, but I (tgl) failed to notice that the new if-tests guarding those calls were really unnecessary, because they're inside a larger if-block checking the same thing. Ranier Vilela Discussion: https://postgr.es/m/MN2PR18MB2927850AB00CF39CC370D107E34B0@MN2PR18MB2927.namprd18.prod.outlook.com
2019-10-19Fix most -Wundef warningsPeter Eisentraut
In some cases #if was used instead of #ifdef in an inconsistent style. Cleaning this up also helps when analyzing cases like 38d8dce61fff09daae0edb6bcdd42b0c7f10ebcd where this makes a difference. There are no behavior changes here, but the change in pg_bswap.h would prevent possible accidental misuse by third-party code. Discussion: https://www.postgresql.org/message-id/flat/3b615ca5-c595-3f1d-fdf7-a429e564f614%402ndquadrant.com
2019-05-22Phase 2 pgindent run for v12.Tom Lane
Switch to 2.1 version of pg_bsd_indent. This formats multiline function declarations "correctly", that is with additional lines of parameter declarations indented to match where the first line's left parenthesis is. Discussion: https://postgr.es/m/CAEepm=0P3FeTXRcU5B2W3jv3PgRVZ-kGUXLGfd42FFhUROO3ug@mail.gmail.com
2019-01-02Update copyright for 2019Bruce Momjian
Backpatch-through: certain files through 9.4
2018-12-06Improve our response to invalid format strings, and detect more cases.Tom Lane
Places that are testing for *printf failure ought to include the format string in their error reports, since bad-format-string is one of the more likely causes of such failure. This both makes it easier to find and repair the mistake, and provides at least some useful info to the user who stumbles across such a problem. Also, tighten snprintf.c to report EINVAL for an invalid flag or final character in a format %-spec (including the case where the %-spec is missing a final character altogether). This seems like better project policy, and it also allows removing an instruction or two from the hot code path. Back-patch the error reporting change in pvsnprintf, since it should be harmless and may be helpful; but not the snprintf.c change. Per discussion of bug #15511 from Ertuğrul Kahveci, which reported an invalid translated format string. These changes don't fix that error, but they should improve matters next time we make such a mistake. Discussion: https://postgr.es/m/15511-1d8b6a0bc874112f@postgresql.org
2018-11-10Disable MSVC warning caused by recent snprintf.c changesAndrew Dunstan
Discussion: https://postgr.es/m/05f348de-0c79-d88d-69b7-434ef828bd4d@2ndQuadrant.com
2018-10-12Make float exponent output on Windows look the same as elsewhere.Tom Lane
Windows, alone among our supported platforms, likes to emit three-digit exponent fields even when two digits would do. Adjust such results to look like the way everyone else does it. Eliminate a bunch of variant expected-output files that were needed only because of this quirk. Discussion: https://postgr.es/m/2934.1539122454@sss.pgh.pa.us
2018-10-08Fix omissions in snprintf.c's coverage of standard *printf functions.Tom Lane
A warning on a NetBSD box revealed to me that pg_waldump/compat.c is using vprintf(), which snprintf.c did not provide coverage for. This is not good if we want to have uniform *printf behavior, and it's pretty silly to omit when it's a one-line function. I also noted that snprintf.c has pg_vsprintf() but for some reason it was not exposed to the outside world, creating another way in which code might accidentally invoke the platform *printf family. Let's just make sure that we replace all eight of the POSIX-standard printf family. Also, upgrade plperl.h and plpython.h to make sure that they do their undefine/redefine rain dance for all eight, not some random maybe-sufficient subset thereof.
2018-10-08Improve snprintf.c's handling of NaN, Infinity, and minus zero.Tom Lane
Up to now, float4out/float8out handled NaN and Infinity cases explicitly, and invoked psprintf only for ordinary float values. This was done because platform implementations of snprintf produce varying representations of these special cases. But now that we use snprintf.c always, it's better to give it the responsibility to produce a uniform representation of these cases, so that we have uniformity across the board not only in float4out/float8out. Hence, move that work into fmtfloat(). Also, teach fmtfloat() to recognize IEEE minus zero and handle it correctly. The previous coding worked only accidentally, and would fail for e.g. "%+f" format (it'd print "+-0.00000"). Now that we're using snprintf.c everywhere, it's not acceptable for it to do weird things in corner cases. (This incidentally avoids a portability problem we've seen on some really ancient platforms, that native sprintf does the wrong thing with minus zero.) Also, introduce a new entry point in snprintf.c to allow float[48]out to bypass the work of interpreting a well-known format spec, as well as bypassing the overhead of the psprintf layer. I modeled this API loosely on strfromd(). In my testing, this brings float[48]out back to approximately the same speed they had when using native snprintf, fixing one of the main performance issues caused by using snprintf.c. (There is some talk of more aggressive work to improve the speed of floating-point output conversion, but these changes seem to provide a better starting point for such work anyway.) Getting rid of the previous ad-hoc hack for Infinity/NaN in fmtfloat() allows removing <ctype.h> from snprintf.c's #includes. I also removed a few other #includes that I think are historical, though the buildfarm may expose that as wrong. Discussion: https://postgr.es/m/13178.1538794717@sss.pgh.pa.us
2018-10-03Replace uint64 use introduced in 4868e446859 in light of 595a0eab7f42.Andres Freund
Reported-By: Tom Lane Discussion: https://postgr.es/m/527.1538598263@sss.pgh.pa.us
2018-10-03Ensure that snprintf.c's fmtint() doesn't overflow when printing INT64_MIN.Andres Freund
This isn't actually a live bug, as the output happens to be the same. But it upsets tools like UBSan, which makes it worthwhile to fix. As it's an issue without practical consequences, don't backpatch. Author: Andres Freund Discussion: https://postgr.es/m/20180928001121.hhx5n6dsygqxr5wu@alap3.anarazel.de
2018-10-03Rationalize snprintf.c's handling of "ll" formats.Tom Lane
Although all known platforms define "long long" as 64 bits, it still feels a bit shaky to be using "va_arg(args, int64)" to pull out an argument that the caller thought was declared "long long". The reason it was coded like this, way back in commit 3311c7669, was to work around the possibility that the compiler had no type named "long long" --- and, at the time, that it maybe didn't have 64-bit ints at all. Now that we're requiring compilers to support C99, those concerns are moot. Let's make the code clearer and more bulletproof by writing "long long" where we mean "long long". This does introduce a hazard that we'd inefficiently use 128-bit arithmetic to convert plain old integers. The way to tackle that would be to provide two versions of fmtint(), one for "long long" and one for narrower types. Since, as of today, no platforms require that, we won't bother with the extra code for now. Discussion: https://postgr.es/m/1680.1538587115@sss.pgh.pa.us
2018-10-03Provide fast path in snprintf.c for conversion specs that are just "%s".Tom Lane
This case occurs often enough (around 45% of conversion specs executed in our regression tests are just "%s") that it's worth an extra test per conversion spec to allow skipping all the logic associated with field widths and padding when it happens. Discussion: https://postgr.es/m/26193.1538582367@sss.pgh.pa.us
2018-10-03Make assorted performance improvements in snprintf.c.Tom Lane
In combination, these changes make our version of snprintf as fast or faster than most platforms' native snprintf, except for cases involving floating-point conversion (which we still delegate to the native sprintf). The speed penalty for a float conversion is down to around 10% though, much better than before. Notable changes: * Rather than always parsing the format twice to see if it contains instances of %n$, do the extra scan only if we actually find a $. This obviously wins for non-localized formats, and even when there is use of %n$, we can avoid scanning text before the first % twice. * Use strchrnul() if available to find the next %, and emit the literal text between % escapes as strings rather than char-by-char. * Create a bespoke function (dopr_outchmulti) for the common case of emitting N copies of the same character, in place of writing loops around dopr_outch. * Simplify construction of the format string for invocations of sprintf for floats. * Const-ify some internal functions, and avoid unnecessary use of pass-by-reference arguments. Patch by me, reviewed by Andres Freund Discussion: https://postgr.es/m/11787.1534530779@sss.pgh.pa.us
2018-10-02Set snprintf.c's maximum number of NL arguments to be 31.Tom Lane
Previously, we used the platform's NL_ARGMAX if any, otherwise 16. The trouble with this is that the platform value is hugely variable, ranging from the POSIX-minimum 9 to as much as 64K on recent FreeBSD. Values of more than a dozen or two have no practical use and slow down the initialization of the argtypes array. Worse, they cause snprintf.c to consume far more stack space than was the design intention, possibly resulting in stack-overflow crashes. Standardize on 31, which is comfortably more than we need (it looks like no existing translatable message has more than about 10 parameters). I chose that, not 32, to make the array sizes powers of 2, for some possible small gain in speed of the memset. The lack of reported crashes suggests that the set of platforms we use snprintf.c on (in released branches) may have no overlap with the set where NL_ARGMAX has unreasonably large values. But that's not entirely clear, so back-patch to all supported branches. Per report from Mateusz Guzik (via Thomas Munro). Discussion: https://postgr.es/m/CAEepm=3VF=PUp2f8gU8fgZB22yPE_KBS0+e1AHAtQ=09schTHg@mail.gmail.com
2018-09-26Implement %m in src/port/snprintf.c, and teach elog.c to rely on that.Tom Lane
I started out with the idea that we needed to detect use of %m format specs in contexts other than elog/ereport calls, because we couldn't rely on that working in *printf calls. But a better answer is to fix things so that it does work. Now that we're using snprintf.c all the time, we can implement %m in that and we've fixed the problem. This requires also adjusting our various printf-wrapping functions so that they ensure "errno" is preserved when they call snprintf.c. Remove elog.c's handmade implementation of %m, and let it rely on snprintf to support the feature. That should provide some performance gain, though I've not attempted to measure it. There are a lot of places where we could now simplify 'printf("%s", strerror(errno))' into 'printf("%m")', but I'm not in any big hurry to make that happen. Patch by me, reviewed by Michael Paquier Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us
2018-08-15Make snprintf.c follow the C99 standard for snprintf's result value.Tom Lane
C99 says that the result should be the number of bytes that would have been emitted given a large enough buffer, not the number we actually were able to put in the buffer. It's time to make our substitute implementation comply with that. Not doing so results in inefficiency in buffer-enlargement cases, and also poses a portability hazard for third-party code that might expect C99-compliant snprintf behavior within Postgres. In passing, remove useless tests for str == NULL; neither C99 nor predecessor standards ever allowed that except when count == 0, so I see no reason to expend cycles on making that a non-crash case for this implementation. Also, don't waste a byte in pg_vfprintf's local I/O buffer; this might have performance benefits by allowing aligned writes during flushbuffer calls. Discussion: https://postgr.es/m/17245.1534289329@sss.pgh.pa.us
2018-07-11Rethink how to get float.h in old Windows API for isnan/isinfAlvaro Herrera
We include <float.h> in every place that needs isnan(), because MSVC used to require it. However, since MSVC 2013 that's no longer necessary (cf. commit cec8394b5ccd), so we can retire the inclusion to a version-specific stanza in win32_port.h, where it doesn't need to pollute random .c files. The header is of course still needed in a few places for other reasons. I (Álvaro) removed float.h from a few more files than in Emre's original patch. This doesn't break the build in my system, but we'll see what the buildfarm has to say about it all. Author: Emre Hasegeli Discussion: https://postgr.es/m/CAE2gYzyc0+5uG+Cd9-BSL7NKC8LSHLNg1Aq2=8ubjnUwut4_iw@mail.gmail.com
2018-02-14Add an assertion that we don't pass NULL to snprintf("%s").Tom Lane
Per commit e748e902d, we appear to have little or no coverage in the buildfarm of machines that will dump core when asked to printf a null string pointer. Let's try to improve that situation by adding an assertion that will make src/port/snprintf.c behave that way. Since it's just an assertion, it won't break anything in production builds, but it will help developers find this type of oversight. Note that while our buildfarm coverage of machines that use that snprintf implementation is pretty thin on the Unix side (apparently amounting only to gaur/pademelon), all of the MSVC critters use it. Discussion: https://postgr.es/m/156b989dbc6fe7c4d3223cf51da61195@postgrespro.ru
2017-10-10Rewrite strnlen replacement implementation from 8a241792f96.Andres Freund
The previous placement of the fallback implementation in libpgcommon was problematic, because libpqport functions need strnlen functionality. Move replacement into libpgport. Provide strnlen() under its posix name, instead of pg_strnlen(). Fix stupid configure bug, executing the test only when compiled with threading support. Author: Andres Freund Discussion: https://postgr.es/m/E1e1gR2-0005fB-SI@gemulon.postgresql.org
2017-10-09Add pg_strnlen() a portable implementation of strlen.Andres Freund
As the OS version is likely going to be more optimized, fall back to it if available, as detected by configure.
2017-06-21Phase 2 of pgindent updates.Tom Lane
Change pg_bsd_indent to follow upstream rules for placement of comments to the right of code, and remove pgindent hack that caused comments following #endif to not obey the general rule. Commit e3860ffa4dd0dad0dd9eea4be9cc1412373a8c89 wasn't actually using the published version of pg_bsd_indent, but a hacked-up version that tried to minimize the amount of movement of comments to the right of code. The situation of interest is where such a comment has to be moved to the right of its default placement at column 33 because there's code there. BSD indent has always moved right in units of tab stops in such cases --- but in the previous incarnation, indent was working in 8-space tab stops, while now it knows we use 4-space tabs. So the net result is that in about half the cases, such comments are placed one tab stop left of before. This is better all around: it leaves more room on the line for comment text, and it means that in such cases the comment uniformly starts at the next 4-space tab stop after the code, rather than sometimes one and sometimes two tabs after. Also, ensure that comments following #endif are indented the same as comments following other preprocessor commands such as #else. That inconsistency turns out to have been self-inflicted damage from a poorly-thought-through post-indent "fixup" in pgindent. This patch is much less interesting than the first round of indent changes, but also bulkier, so I thought it best to separate the effects. Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
2015-05-19Revert error-throwing wrappers for the printf family of functions.Tom Lane
This reverts commit 16304a013432931e61e623c8d85e9fe24709d9ba, except for its changes in src/port/snprintf.c; as well as commit cac18a76bb6b08f1ecc2a85e46c9d2ab82dd9d23 which is no longer needed. Fujii Masao reported that the previous commit caused failures in psql on OS X, since if one exits the pager program early while viewing a query result, psql sees an EPIPE error from fprintf --- and the wrapper function thought that was reason to panic. (It's a bit surprising that the same does not happen on Linux.) Further discussion among the security list concluded that the risk of other such failures was far too great, and that the one-size-fits-all approach to error handling embodied in the previous patch is unlikely to be workable. This leaves us again exposed to the possibility of the type of failure envisioned in CVE-2015-3166. However, that failure mode is strictly hypothetical at this point: there is no concrete reason to believe that an attacker could trigger information disclosure through the supposed mechanism. In the first place, the attack surface is fairly limited, since so much of what the backend does with format strings goes through stringinfo.c or psprintf(), and those already had adequate defenses. In the second place, even granting that an unprivileged attacker could control the occurrence of ENOMEM with some precision, it's a stretch to believe that he could induce it just where the target buffer contains some valuable information. So we concluded that the risk of non-hypothetical problems induced by the patch greatly outweighs the security risks. We will therefore revert, and instead undertake closer analysis to identify specific calls that may need hardening, rather than attempt a universal solution. We have kept the portion of the previous patch that improved snprintf.c's handling of errors when it calls the platform's sprintf(). That seems to be an unalloyed improvement. Security: CVE-2015-3166
2015-05-18Add error-throwing wrappers for the printf family of functions.Noah Misch
All known standard library implementations of these functions can fail with ENOMEM. A caller neglecting to check for failure would experience missing output, information exposure, or a crash. Check return values within wrappers and code, currently just snprintf.c, that bypasses the wrappers. The wrappers do not return after an error, so their callers need not check. Back-patch to 9.0 (all supported versions). Popular free software standard library implementations do take pains to bypass malloc() in simple cases, but they risk ENOMEM for floating point numbers, positional arguments, large field widths, and large precisions. No specification demands such caution, so this commit regards every call to a printf family function as a potential threat. Injecting the wrappers implicitly is a compromise between patch scope and design goals. I would prefer to edit each call site to name a wrapper explicitly. libpq and the ECPG libraries would, ideally, convey errors to the caller rather than abort(). All that would be painfully invasive for a back-patched security fix, hence this compromise. Security: CVE-2015-3166
2015-05-18Permit use of vsprintf() in PostgreSQL code.Noah Misch
The next commit needs it. Back-patch to 9.0 (all supported versions).
2015-02-04Add missing float.h include to snprintf.c.Andres Freund
On windows _isnan() (which isnan() is redirected to in port/win32.h) is declared in float.h, not math.h. Per buildfarm animal currawong. Backpatch to all supported branches.
2015-02-02port/snprintf(): fix overflow and do paddingBruce Momjian
Prevent port/snprintf() from overflowing its local fixed-size buffer and pad to the desired number of digits with zeros, even if the precision is beyond the ability of the native sprintf(). port/snprintf() is only used on systems that lack a native snprintf(). Reported by Bruce Momjian. Patch by Tom Lane. Backpatch to all supported versions. Security: CVE-2015-0242
2014-05-06pgindent run for 9.4Bruce Momjian
This includes removing tabs after periods in C comments, which was applied to back branches, so this change should not effect backpatching.
2014-01-23Allow use of "z" flag in our printf calls, and use it where appropriate.Tom Lane
Since C99, it's been standard for printf and friends to accept a "z" size modifier, meaning "whatever size size_t has". Up to now we've generally dealt with printing size_t values by explicitly casting them to unsigned long and using the "l" modifier; but this is really the wrong thing on platforms where pointers are wider than longs (such as Win64). So let's start using "z" instead. To ensure we can do that on all platforms, teach src/port/snprintf.c to understand "z", and add a configure test to force use of that implementation when the platform's version doesn't handle "z". Having done that, modify a bunch of places that were using the unsigned-long hack to use "z" instead. This patch doesn't pretend to have gotten everyplace that could benefit, but it catches many of them. I made an effort in particular to ensure that all uses of the same error message text were updated together, so as not to increase the number of translatable strings. It's possible that this change will result in format-string warnings from pre-C99 compilers. We might have to reconsider if there are any popular compilers that will warn about this; but let's start by seeing what the buildfarm thinks. Andres Freund, with a little additional work by me
2011-04-10pgindent run before PG 9.1 beta 1.Bruce Momjian
2010-09-20Remove cvs keywords from all files.Magnus Hagander
2010-07-06pgindent run for 9.0, second runBruce Momjian
2008-03-18Fix our printf implementation to follow spec: if a star parameterTom Lane
value for a precision is negative, act as though precision weren't specified at all, that is the whole .* part of the format spec should be ignored. Our previous coding took it as .0 which is certainly wrong. Per report from Kris Jurka and local testing. Possibly this should be back-patched, but it would be good to get some more testing first; in any case there are no known cases where there's really a problem on the backend side.
2007-03-26Remove advertising clause from Berkeley BSD-licensed files, perBruce Momjian
instructions from Berkeley.