Re: patch: garbage error strings in libpq - Mailing list pgsql-patches
From | jtv@xs4all.nl |
---|---|
Subject | Re: patch: garbage error strings in libpq |
Date | |
Msg-id | 16060.202.47.227.25.1120801262.squirrel@202.47.227.25 Whole thread Raw |
In response to | Re: patch: garbage error strings in libpq (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: patch: garbage error strings in libpq
Re: patch: garbage error strings in libpq |
List | pgsql-patches |
Tom Lane wrote: > > (1) The fact that gettext works at all seems to me to be sufficient > empirical evidence that it's a working solution. Tom, you have GOT to be joking. I agree with some of the things you say (see below), but here you're just not making sense. Consider: 1. We're only talking about a very specific property of gettext(), namely restoration of errno in calls that are not ordered w.r.t. a use of errno. This is only relevant in a very limited number of all calls to gettext(), so the mere fact that gettext() works doesn't prove anything relevant. 2. In those limited few cases, and for this specific property, we already know that something in this general area is *not* working, so you'd have to find, fix and test that before you could even make this argument. 3. Given that a call to gettext() crosses not just object-file but actual library boundaries, what do you think the chances are--regardless of language garantees and compiler aggressiveness--that we'd see the compiler interleaving instructions from gettext() with a use of errno on the other side of that boundary? Does the assumption that gettext() works at all, therefore, make a reliable indication that there is no problem? 4. In the case of libpq_gettext(), we're not crossing library boundaries. We're not even crossing object-file boundaries in some cases. That makes the chances of instructions being scheduled across a call much greater--and the question about sequence points much more relevant. 5. From what I understand, the latest versions of gcc are actually beginning to schedule instructions across function call boundaries. That will undoubtedly increase in the future. There is even a new feature that can, as far as I can see, lead to instruction scheduling across source file boundaries. > (2) Whether there are > sequence points in the function call or not, there most certainly are > sequence points inside each called function. The spec allows the > functions involved to be called in an unspecified order, but it doesn't > allow the compiler to interleave the execution of several functions. That would answer the big question here, but where does it say that? I saw Neil's point that the sequence points before function calls apply for the nested calls as well as the outer one, but there is no ordering between those "nested-call" sequence points. It's all easy when you have a total ordering, but we're in a partial ordering here. So the missing piece of the puzzle is still that same question. Obviously the compiler isn't allowed to interleave function executions (or at least not let you find out about it) if there is a sequence point _between_ them. There is in sequential calls, for example, because there is a sequence point before the function is entered. But what happens if there isn't a separating sequence point, like here? > (3) Interpretation or not, the approach that Jeroen proposes is > unmaintainable; people will not remember to use such a kluge everywhere > they'd need to. I'd much rather fix it in one place and do whatever we > have to do to keep the compiler from breaking that one place. That means you haven't seen the approach I suggested the day before yesterday, where I also explained that I felt it best to fix the incumbent bugs first before refactoring the result. You're still talking about my initial quick-fix patch, which IMHO could have gone in already while we were arguing over a long-term solution. That patch was ugly solely in the sense that the original broken code was ugly; if you want to use words like "kluge" then please direct them there. The copy-paste style of writing loops didn't help either. As I suggested on Wednesday, maybe we can wrap the combination of printfPQExpBuffer() and libpq_gettext() into a single function that takes a PGconn *, a format string, and varargs. This makes the calls a lot shorter and simpler, it moots the question of sequence points because errno would be the first thing to be evaluated, and en passant we'll fix a few cases where we forgot to call libpq_gettext(). We'd have to have a similar wrapper for snprintf(), but then I think all cases in libpq are covered. Jeroen
pgsql-patches by date: