Re: check_strxfrm_bug()

2023-07-10 Thread Thomas Munro
On Tue, Jul 11, 2023 at 2:28 AM Peter Eisentraut wrote: > This looks sensible to me. > > If we ever need mbstowcs_l() etc. outside of pg_locale.c, then the > proper way would be to make a mbstowcs_l.c file in src/port/. > > But I like your approach for now because it moves us more firmly into >

Re: check_strxfrm_bug()

2023-07-10 Thread Peter Eisentraut
On 10.07.23 04:51, Thomas Munro wrote: On Sun, Jul 9, 2023 at 6:35 PM Thomas Munro wrote: On Sun, Jul 9, 2023 at 6:20 PM Peter Eisentraut wrote: So I don't think this code is correct. AFAICT, there is nothing right now that can possibly define HAVE_MBSTOWCS_L on Windows/MSVC. Was that the

Re: check_strxfrm_bug()

2023-07-09 Thread Thomas Munro
On Sun, Jul 9, 2023 at 6:35 PM Thomas Munro wrote: > On Sun, Jul 9, 2023 at 6:20 PM Peter Eisentraut wrote: > > So I don't think this code is correct. AFAICT, there is nothing right > > now that can possibly define HAVE_MBSTOWCS_L on Windows/MSVC. Was that > > the intention? > > Yes, that was

Re: check_strxfrm_bug()

2023-07-09 Thread Thomas Munro
On Sun, Jul 9, 2023 at 6:20 PM Peter Eisentraut wrote: > So I don't think this code is correct. AFAICT, there is nothing right > now that can possibly define HAVE_MBSTOWCS_L on Windows/MSVC. Was that > the intention? Yes, that was my intention. Windows actually doesn't have them. The

Re: check_strxfrm_bug()

2023-07-09 Thread Peter Eisentraut
On 07.07.23 22:30, Thomas Munro wrote: Thinking about how to bring this all into "normal" form -- where HAVE_XXX means "system defines XXX", not "system defines XXX || we define a replacement" HAVE_XXX means "code can use XXX", doesn't matter how it got there (it could also be a libpgport

Re: check_strxfrm_bug()

2023-07-08 Thread Thomas Munro
On Sat, Jul 8, 2023 at 10:13 AM Thomas Munro wrote: > Thanks. I will wait a bit to see if Peter has any other comments and > then push this. I haven't actually tested with Solution.pm due to > lack of CI for that, but fingers crossed, since the build systems will > now agree, reducing the

Re: check_strxfrm_bug()

2023-07-07 Thread Thomas Munro
On Sat, Jul 8, 2023 at 8:52 AM Tristan Partin wrote: > Should you wrap the two _l function replacements in HAVE_USELOCALE > instead of WIN32? I find that more confusing, and I'm also not sure if HAVE_USELOCALE is even going to survive (based on your nearby thread). I mean, by the usual criteria

Re: check_strxfrm_bug()

2023-07-07 Thread Tristan Partin
On Fri Jul 7, 2023 at 3:30 PM CDT, Thomas Munro wrote: > On Fri, Jul 7, 2023 at 4:20 AM Peter Eisentraut wrote: > > I think the correct solution is to set HAVE_MBSTOWCS_L in Solution.pm. > > Compare HAVE_FSEEKO, which is set in Solution.pm with fseeko being > > defined in win32_port.h. > > In

Re: check_strxfrm_bug()

2023-07-07 Thread Thomas Munro
On Fri, Jul 7, 2023 at 4:20 AM Peter Eisentraut wrote: > I think the correct solution is to set HAVE_MBSTOWCS_L in Solution.pm. > Compare HAVE_FSEEKO, which is set in Solution.pm with fseeko being > defined in win32_port.h. In this version I have it in there, but set it to undef. This way

Re: check_strxfrm_bug()

2023-07-06 Thread Peter Eisentraut
On 05.07.23 00:15, Thomas Munro wrote: On Tue, Jul 4, 2023 at 2:52 AM Tristan Partin wrote: The patch looks good to me as well. Happy to rebase my other patch on this one. Thanks. Here is a slightly tidier version. It passes on CI[1] including the optional extra MinGW64/Meson task, and the

Re: check_strxfrm_bug()

2023-07-04 Thread Thomas Munro
On Wed, Jul 5, 2023 at 10:15 AM Thomas Munro wrote: > [1] https://cirrus-ci.com/build/5298278007308288 That'll teach me to be impatient. I only waited for compiling to finish after triggering the optional extra MinGW task before sending the above email, figuring that the only risk was there,

Re: check_strxfrm_bug()

2023-07-04 Thread Thomas Munro
On Tue, Jul 4, 2023 at 2:52 AM Tristan Partin wrote: > The patch looks good to me as well. Happy to rebase my other patch on > this one. Thanks. Here is a slightly tidier version. It passes on CI[1] including the optional extra MinGW64/Meson task, and the MinGW64/autoconf configure+build that

Re: check_strxfrm_bug()

2023-07-03 Thread Tristan Partin
On Sun Jul 2, 2023 at 8:49 PM CDT, Thomas Munro wrote: > On Sun, Jul 2, 2023 at 4:25 AM Noah Misch wrote: > > On Wed, Jun 28, 2023 at 01:02:21PM +1200, Thomas Munro wrote: > > > On Wed, Jun 28, 2023 at 11:03 AM Thomas Munro > > > wrote: > > > > The GCC build farm has just received some SPARC

Re: check_strxfrm_bug()

2023-07-02 Thread Thomas Munro
On Sun, Jul 2, 2023 at 4:25 AM Noah Misch wrote: > On Wed, Jun 28, 2023 at 01:02:21PM +1200, Thomas Munro wrote: > > On Wed, Jun 28, 2023 at 11:03 AM Thomas Munro > > wrote: > > > The GCC build farm has just received some SPARC hardware new enough to > > > run modern Solaris (hostname gcc106),

Re: check_strxfrm_bug()

2023-07-01 Thread Noah Misch
On Wed, Jun 28, 2023 at 01:02:21PM +1200, Thomas Munro wrote: > On Wed, Jun 28, 2023 at 11:03 AM Thomas Munro wrote: > > The GCC build farm has just received some SPARC hardware new enough to > > run modern Solaris (hostname gcc106), so if wrasse were moved over > > there we could finally assume

Re: check_strxfrm_bug()

2023-06-27 Thread Thomas Munro
On Wed, Jun 28, 2023 at 11:03 AM Thomas Munro wrote: > The GCC build farm has just received some SPARC hardware new enough to > run modern Solaris (hostname gcc106), so if wrasse were moved over > there we could finally assume all systems have POSIX 2008 (AKA > SUSv4)'s locale_t. That would look

Re: check_strxfrm_bug()

2023-06-27 Thread Thomas Munro
The GCC build farm has just received some SPARC hardware new enough to run modern Solaris (hostname gcc106), so if wrasse were moved over there we could finally assume all systems have POSIX 2008 (AKA SUSv4)'s locale_t. It's slightly annoying that Windows has locale_t but doesn't have

Re: check_strxfrm_bug()

2023-06-12 Thread Heikki Linnakangas
On 12/06/2023 01:15, Thomas Munro wrote: There are still at least a couple of functions that lack XXX_l variants in the standard: mbstowcs() and wcstombs() (though we use the non-standard _l variants if we find them in ), but that's OK because we use uselocale() and not setlocale(), because

Re: check_strxfrm_bug()

2023-06-11 Thread Thomas Munro
On Mon, Apr 17, 2023 at 8:00 AM Thomas Munro wrote: > On Sun, Dec 18, 2022 at 10:27 AM Thomas Munro wrote: > > With my garbage collector hat on, that made me wonder if there was > > some more potential cleanup here: could we require locale_t yet? The > > last straggler systems on our target OS

Re: check_strxfrm_bug()

2023-04-22 Thread Jonathan S. Katz
On 4/19/23 9:34 PM, Thomas Munro wrote: On Wed, Apr 19, 2023 at 2:31 PM Jonathan S. Katz wrote: To be clear, is the proposal to remove both "check_strxfrm_bug" and "TRUST_STRXFRM"? Given a bunch of folks who have expertise in this area of code all agree with removi

Re: check_strxfrm_bug()

2023-04-20 Thread Jeff Davis
On Thu, 2023-04-20 at 13:34 +1200, Thomas Munro wrote: > I could write a patch to remove the libc strxfrm support, but since > Jeff recently wrote new code in 16 to abstract that stuff, he might > prefer to look at it? +1 to removing it. As far as how it's removed, we could directly check: if

Re: check_strxfrm_bug()

2023-04-19 Thread Thomas Munro
On Wed, Apr 19, 2023 at 2:31 PM Jonathan S. Katz wrote: > To be clear, is the proposal to remove both "check_strxfrm_bug" and > "TRUST_STRXFRM"? > > Given a bunch of folks who have expertise in this area of code all agree > with removing the above as part

Re: check_strxfrm_bug()

2023-04-19 Thread Tom Lane
Joe Conway writes: > I have wondered a few times, given the known issues with strxfrm, how is > the use in selfuncs.c still ok. Has anyone taken a hard look at that? On the one hand, we only need approximately-correct results in that code. On the other, the result is fed to

Re: check_strxfrm_bug()

2023-04-19 Thread Joe Conway
On 4/18/23 21:19, Thomas Munro wrote: On Tue, Apr 18, 2023 at 11:52 AM Michael Paquier wrote: On Mon, Apr 17, 2023 at 03:40:14PM -0700, Peter Geoghegan wrote: > +1 for getting rid of TRUST_STRXFRM. +1 The situation is not improving fast, and requires hard work to follow on each OS.

Re: check_strxfrm_bug()

2023-04-18 Thread Jonathan S. Katz
. Clearly, mainstreaming ICU is the way to go. libc support will always have niche uses, to be compatible with other software on the box, but trusting strxfrm doesn't seem to be on the cards any time soon. [RMT hat, personal opinion on RMT] To be clear, is the proposal to remove both "check_strxfr

Re: check_strxfrm_bug()

2023-04-18 Thread Thomas Munro
On Tue, Apr 18, 2023 at 11:52 AM Michael Paquier wrote: > On Mon, Apr 17, 2023 at 03:40:14PM -0700, Peter Geoghegan wrote: > > +1 for getting rid of TRUST_STRXFRM. +1 The situation is not improving fast, and requires hard work to follow on each OS. Clearly, mainstreaming ICU is the way to go.

Re: check_strxfrm_bug()

2023-04-17 Thread Michael Paquier
On Mon, Apr 17, 2023 at 03:40:14PM -0700, Peter Geoghegan wrote: > On Mon, Apr 17, 2023 at 2:48 PM Tom Lane wrote: >> +1. I wonder if we should go further and get rid of TRUST_STRXFRM >> and the not-so-trivial amount of code around it (pg_strxfrm_enabled >> etc). Carrying that indefinitely in

Re: check_strxfrm_bug()

2023-04-17 Thread Peter Geoghegan
On Mon, Apr 17, 2023 at 2:48 PM Tom Lane wrote: > +1. I wonder if we should go further and get rid of TRUST_STRXFRM > and the not-so-trivial amount of code around it (pg_strxfrm_enabled > etc). Carrying that indefinitely in the probably-vain hope that > the libraries will become trustworthy

Re: check_strxfrm_bug()

2023-04-17 Thread Tom Lane
Nathan Bossart writes: > On Thu, Dec 15, 2022 at 03:22:59PM +1300, Thomas Munro wrote: >> While studying Jeff's new crop of collation patches I noticed in >> passing that check_strxfrm_bug() must surely by now be unnecessary. >> The buffer overrun bugs were fixed a decade

Re: check_strxfrm_bug()

2023-04-17 Thread Nathan Bossart
On Thu, Dec 15, 2022 at 03:22:59PM +1300, Thomas Munro wrote: > While studying Jeff's new crop of collation patches I noticed in > passing that check_strxfrm_bug() must surely by now be unnecessary. > The buffer overrun bugs were fixed a decade ago, and the relevant > systems

Re: check_strxfrm_bug()

2023-04-16 Thread Thomas Munro
On Sun, Dec 18, 2022 at 10:27 AM Thomas Munro wrote: > With my garbage collector hat on, that made me wonder if there was > some more potential cleanup here: could we require locale_t yet? The > last straggler systems on our target OS list to add the POSIX locale_t > stuff were Solaris 11.4

Re: check_strxfrm_bug()

2022-12-17 Thread Thomas Munro
On Thu, Dec 15, 2022 at 3:22 PM Thomas Munro wrote: > ... If you're worried that the bugs might > come back, then the test is insufficient: modern versions of both OSes > have strxfrm_l(), which we aren't checking. With my garbage collector hat on, that made me wonder if there was some more

check_strxfrm_bug()

2022-12-14 Thread Thomas Munro
Hi While studying Jeff's new crop of collation patches I noticed in passing that check_strxfrm_bug() must surely by now be unnecessary. The buffer overrun bugs were fixed a decade ago, and the relevant systems are way out of support. If you're worried that the bugs might come back, then the test