oid failures on Alpha solved
Hi,
It turns out the problem causing the oid failures is with our
snprintf. specifically we are formatting "%u" incorrectly:
using a enhanced-for-testing version of our snprintf I get.
formatting '-1040' with '%lu'
snprintf = 18446744073709550576
sprintf = 18446744073709550576
formatting '-1040' with '%u'
snprintf = 18446744073709550576
sprintf = 4294966256
oidout() is where the offending call originates, FWIW.
snprintf(result, 12, "%u", o);
I've massaged in the snprintf.c code from openssh into postgres, and
oid now passes the regression test, but have a couple of questions:
1) could the openssh code be a candidate to replace our version? It
looks quite a bit more 'featureful', and I'd imagine it is about
as safe as snprintf gets.
2) do we _need_ oidout() to "%u", or could we "%lu" and fully take
advantage of the longer long on 64bit platforms?
cheers.
brent
Brent Verner <brent@rcfile.org> writes:
formatting '-1040' with '%u'
snprintf = 18446744073709550576
sprintf = 4294966256
oidout() is where the offending call originates, FWIW.
snprintf(result, 12, "%u", o);
Hm. This is clearly a bug in snprintf. Did I understand you correctly
that configure is choosing backend/port/snprintf.c rather than one from
the C library? If so, it should be straightforward to fix.
1) could the openssh code be a candidate to replace our version? It
looks quite a bit more 'featureful', and I'd imagine it is about
as safe as snprintf gets.
(a) what's the license? (b) is it a lot bigger than the one we have?
2) do we _need_ oidout() to "%u", or could we "%lu" and fully take
advantage of the longer long on 64bit platforms?
OID is 4 bytes and %u is the correct format for it. 8-byte OIDs are a
can of worms that we will *not* open just now.
regards, tom lane
Brent Verner <brent@rcfile.org> writes:
formatting '-1040' with '%u'
snprintf = 18446744073709550576
sprintf = 4294966256
oidout() is where the offending call originates, FWIW.
snprintf(result, 12, "%u", o);
Hm. This is clearly a bug in snprintf.
OK, I was able to duplicate this failure on Debian Alpha after forcing
our snprintf emulation to be used instead of the system's. Problem is
sloppy signed-vs-unsigned casting in snprintf.c. I've committed a fix.
Another day, another bug...
regards, tom lane
On 30 Dec 2000 at 12:57 (-0500), Tom Lane wrote:
| Brent Verner <brent@rcfile.org> writes:
| > formatting '-1040' with '%u'
| > snprintf = 18446744073709550576
| > sprintf = 4294966256
|
| > oidout() is where the offending call originates, FWIW.
| > snprintf(result, 12, "%u", o);
|
| Hm. This is clearly a bug in snprintf. Did I understand you correctly
| that configure is choosing backend/port/snprintf.c rather than one from
| the C library? If so, it should be straightforward to fix.
yes, backend/port/snprintf.c is being used.
| > 1) could the openssh code be a candidate to replace our version? It
| > looks quite a bit more 'featureful', and I'd imagine it is about
| > as safe as snprintf gets.
|
| (a) what's the license? (b) is it a lot bigger than the one we have?
a) no license in file. file is obviously 'related' to the current
version used in postgresql. version I used came with openssh-2.?,
so if any license is applicable, i'd imagine it's BSD-like.
b) 10.7k postgresql snprintf.c
18.4k openssh snprintf.c
(a sizable portion of this size difference is a test main() at the
end of the file openssh file, which I found useful)
you can snag a slightly modified-to-work-in-postgres copy from
http://pg.rcfile.org/alpha/snprintf.c if you'd like to check it out.
it is a drop in replacement as it stands. my changes to the original
were s/long double/double/g and moving some #cpp directives around.
| > 2) do we _need_ oidout() to "%u", or could we "%lu" and fully take
| > advantage of the longer long on 64bit platforms?
|
| OID is 4 bytes and %u is the correct format for it. 8-byte OIDs are a
| can of worms that we will *not* open just now.
right on.
cheers.
brent
On 30 Dec 2000 at 14:24 (-0500), Tom Lane wrote:
| > Brent Verner <brent@rcfile.org> writes:
| >> formatting '-1040' with '%u'
| >> snprintf = 18446744073709550576
| >> sprintf = 4294966256
|
| >> oidout() is where the offending call originates, FWIW.
| >> snprintf(result, 12, "%u", o);
|
| > Hm. This is clearly a bug in snprintf.
|
| OK, I was able to duplicate this failure on Debian Alpha after forcing
| our snprintf emulation to be used instead of the system's. Problem is
| sloppy signed-vs-unsigned casting in snprintf.c. I've committed a fix.
| Another day, another bug...
indeed. the most recent snprintf.c works as expected.
thank you.
brent