Thread: OID from insert has extra letter
This is very odd. I have a database app failing sometimes with an error
when I try to get back my inserted row. I do this to get all the fields
that were adjusted for defaults, triggers and rules. On an insert query
I expect to get the OID of the just inserted tuple. Once in a while I
get the wrong value. Here are two examples.
Get: 31930584, should be 3193058
Get: 31943386, should be 3194338
As you can see, there is an extra digit added. I am using PQoidStatus()
to get the OID. Is this the right way? Is it possible that there is
a buffer/terminating problem in the insert code? I assume that most
people are not examining this value so it may be that something slipped
through. Anyone else ever seen this?
--
D'Arcy J.M. Cain <darcy@{druid|vex}.net> | Democracy is three wolves
http://www.druid.net/darcy/ | and a sheep voting on
+1 416 425 1212 (DoD#0082) (eNTP) | what's for dinner.
darcy@druid.net (D'Arcy J.M. Cain) writes:
> Get: 31930584, should be 3193058
> Get: 31943386, should be 3194338
> As you can see, there is an extra digit added. I am using PQoidStatus()
> to get the OID. Is this the right way? Is it possible that there is
> a buffer/terminating problem in the insert code?
Yes, on looking at it I see that someone broke PQoidStatus() in 7.0.
If you want to fix your copy, the patch (line numbers are for current
CVS) is
Index: fe-exec.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.98
retrieving revision 1.100
diff -c -r1.98 -r1.100
*** fe-exec.c 2001/01/24 19:43:30 1.98
--- fe-exec.c 2001/02/06 02:02:27 1.100
***************
*** 2035,2041 **** if (len > 23) len = 23; strncpy(buf, res->cmdStatus + 7, len);
! buf[23] = '\0'; return buf; }
--- 2035,2041 ---- if (len > 23) len = 23; strncpy(buf, res->cmdStatus + 7, len);
! buf[len] = '\0'; return buf; }
However, really I'd recommend using PQoidValue() and not PQoidStatus()
at all.
regards, tom lane
On Mon, Feb 05, 2001 at 09:17:45PM -0500, Tom Lane wrote: > > Yes, on looking at it I see that someone broke PQoidStatus() in 7.0. > If you want to fix your copy, the patch (line numbers are for current > CVS) is > > Index: fe-exec.c > =================================================================== > RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v > retrieving revision 1.98 > retrieving revision 1.100 > diff -c -r1.98 -r1.100 > *** fe-exec.c 2001/01/24 19:43:30 1.98 > --- fe-exec.c 2001/02/06 02:02:27 1.100 > *************** > *** 2035,2041 **** > if (len > 23) > len = 23; > strncpy(buf, res->cmdStatus + 7, len); > ! buf[23] = '\0'; > > return buf; > } > --- 2035,2041 ---- > if (len > 23) > len = 23; > strncpy(buf, res->cmdStatus + 7, len); > ! buf[len] = '\0'; > > return buf; > } > Hmm, is there some undocumented feature of strncpy that I don't know about, where it modifies the passed length variable (which would be hard, since it's pass by value)? Otherwise, doesn't this patch just replace the constant '23' with the variable 'len', set to 23? Ross
> > *** fe-exec.c 2001/01/24 19:43:30 1.98 > > --- fe-exec.c 2001/02/06 02:02:27 1.100 > > *************** > > *** 2035,2041 **** > > if (len > 23) > > len = 23; > > strncpy(buf, res->cmdStatus + 7, len); > > ! buf[23] = '\0'; > > > > return buf; > > } > > --- 2035,2041 ---- > > if (len > 23) > > len = 23; > > strncpy(buf, res->cmdStatus + 7, len); > > ! buf[len] = '\0'; > > > > return buf; > > } > > > > Hmm, is there some undocumented feature of strncpy that I don't know > about, where it modifies the passed length variable (which would be hard, > since it's pass by value)? Otherwise, doesn't this patch just replace > the constant '23' with the variable 'len', set to 23? What if len < 23? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
On Tue, Feb 06, 2001 at 01:21:00PM -0500, Bruce Momjian wrote: > What if len < 23? mea culpa. Must go eat lunch. No sugar to brain. (and no, I didn't put the original error in :-) Ross
On Tue, Feb 06, 2001 at 01:21:00PM -0500, Bruce Momjian wrote:
> > > *** fe-exec.c 2001/01/24 19:43:30 1.98
> > > --- fe-exec.c 2001/02/06 02:02:27 1.100
> > > ***************
> > > *** 2035,2041 ****
> > > if (len > 23)
> > > len = 23;
> > > strncpy(buf, res->cmdStatus + 7, len);
> > > ! buf[23] = '\0';
> > >
> > > return buf;
> > > }
> > > --- 2035,2041 ----
> > > if (len > 23)
> > > len = 23;
> > > strncpy(buf, res->cmdStatus + 7, len);
> > > ! buf[len] = '\0';
> > >
> > > return buf;
> > > }
> > >
> >
> > Hmm, is there some undocumented feature of strncpy that I don't know
> > about, where it modifies the passed length variable (which would be hard,
> > since it's pass by value)? Otherwise, doesn't this patch just replace
> > the constant '23' with the variable 'len', set to 23?
>
> What if len < 23?
If len < 23, then strncpy will have terminated the destination
already. Poking out buf[23] just compensates for a particular
bit of brain damage in strncpy. Read the man page:
The strncpy() function is similar [to strcpy], except that not more than n bytes of src are copied. Thus, if
thereis no null byte among the first n bytes of src, the result wil not be null-terminated.
Thus, the original code is OK, except probably the literal "23"
in place of what should be a meaningful symbolic constant, or
(at least!) sizeof(buf) - 1.
BTW, that static buffer in PGoidStatus is likely to upset threaded
client code...
<ob-ed>
To null-terminate strings is an Abomination.
</ob-ed>
Nathan Myers
ncm@zembu.com
ncm@zembu.com (Nathan Myers) writes:
> Thus, the original code is OK, except probably the literal "23"
> in place of what should be a meaningful symbolic constant, or
> (at least!) sizeof(buf) - 1.
No, the original code is NOT ok. Read the man page again. As the
code stood, the only null that ever got written to the buffer was the
one installed in buf[23]. The only reason it appeared to work at all
was that buf would start out all zeroes --- but after one or more uses
it's not all zeroes anymore. See the bug report that started the
thread ...
> BTW, that static buffer in PGoidStatus is likely to upset threaded
> client code...
Yeah, we know. The routine is deprecated now because of that.
regards, tom lane
On Tue, Feb 06, 2001 at 07:08:20PM -0500, Tom Lane wrote:
> ncm@zembu.com (Nathan Myers) writes:
> > Thus, the original code is OK, except probably the literal "23"
> > in place of what should be a meaningful symbolic constant, or
> > (at least!) sizeof(buf) - 1.
>
> No, the original code is NOT ok. Read the man page again. As the
> code stood, the only null that ever got written to the buffer was the
> one installed in buf[23]. The only reason it appeared to work at all
> was that buf would start out all zeroes --- but after one or more uses
> it's not all zeroes anymore. See the bug report that started the
> thread ...
Seems it's a non-portable behavior:
The strncpy() function is similar, except that not more than n bytes of src are copied. Thus, if there is
no null byte among the first n bytes of src, the result wil not be null-terminated.
In the case where the length of src is less than that of n, the remainder of dest will be padded with
nulls.
I've already forgotten what platform the original bug report came from.
Ross
On Tue, Feb 06, 2001 at 06:17:46PM -0600, Ross J. Reedstrom wrote: > > Seems it's a non-portable behavior: > > The strncpy() function is similar, except that not more > than n bytes of src are copied. Thus, if there is no null > byte among the first n bytes of src, the result wil not be > null-terminated. > > In the case where the length of src is less than that of > n, the remainder of dest will be padded with nulls. > > I've already forgotten what platform the original bug report came from. Just checked, D'Arcy never told us. But it doesn't matter, the manpage lies. I just tested it with a tiny little program that copies two different constant strings into a buffer. Nothing get's padded with nulls, as Tom knew. Once again, experience trumps book knowledge. (Checking a couple random examples from my own code, I seem to have lucked out: I've a habit of zeroing my buffers myself) Ross
"Ross J. Reedstrom" <reedstrm@rice.edu> writes:
> Seems it's a non-portable behavior:
Not at all. The code is asking strncpy to copy n bytes, where n is
known to be <= strlen of the source string. Every spec-conforming
implementation of strncpy will copy n bytes, no more, no less, and
will *not* add a trailing null after them. The only reason the code
appeared to work is that it was dealing with a static buffer that
starts out all zeroes, so the trailing null was already in place.
Until the buffer had been used for a longer OID, that is.
regards, tom lane