Skip to Content.
Sympa Menu

freetds - RE: Losing last character of TEXT fields

freetds AT lists.ibiblio.org

Subject: FreeTDS Development Group

List archive

Chronological Thread  
  • From: "Lowden, James K" <LowdenJK AT bernstein.com>
  • To: "'TDS Development Group'" <freetds AT franklin.metalab.unc.edu>
  • Subject: RE: Losing last character of TEXT fields
  • Date: Mon, 8 Jul 2002 15:22:32 -0400


> From: L J Bayuk [mailto:lbayuk AT mindspring.com]
> Sent: July 7, 2002 9:30 PM
>
> When I select from a TEXT field, the last character of
> the value is lost. (CHAR and VARCHAR are OK). This only happens when
> using a recent FreeTDS "current" CVS snapshot, not when using
> FreeTDS-0.53. It works fine in 0.53.
>
> I think the problem is an off-by-1 error here in
> src/tds/convert.c in the
> function tds_convert_text():
> cplen = srclen > destlen ? destlen : srclen;
> memcpy(dest, src, cplen);
> * dest[cplen-1] = '\0';
> return strlen(dest);

Brian, what do think about this and the proposed patch? The story in cvs is
an unhappy one. I looked at a diff for src/tds/convert.c between version
1.12 and 1.13; there's a little tug of war over whether and when cplen
should be decremented. We seem to be oscillating between truncating the
data and overflowing the buffer. :/ My ten foot pole says not to touch it.


--jkl

> I think that (*) line should be: dest[cplen] = '\0';
> (that's how it was at 0.53) but it seems to me this could
> overflow the dest
> buffer. I browsed the CVS on SourceForge and found that this
> was changed
> in rev 1.13 of that file on May 25, 2002 by Victor K. but it
> isn't clear
> why. I would like to propose the patch below. It changes it back, and
> includes another change to prevent dest buffer overflow, and
> a third change
> in how to get the return value. These make sense to me, and
> seem to work,
> but I'm new with this so dissenting opinions would be welcome...
>
> *** convert.c.bak Tue Jun 25 21:44:27 2002
> --- convert.c Sat Jul 6 10:41:43 2002
> ***************
> *** 122,131 ****
> switch(desttype) {
> case SYBTEXT:
> case SYBCHAR:
> ! cplen = srclen > destlen ? destlen : srclen;
> memcpy(dest, src, cplen);
> ! dest[cplen-1] = '\0';
> ! return strlen(dest);
> case SYBBINARY:
> cplen = srclen > destlen ? destlen : srclen;
> memcpy(dest, src, cplen);
> --- 122,131 ----
> switch(desttype) {
> case SYBTEXT:
> case SYBCHAR:
> ! cplen = srclen >= destlen ? destlen-1 : srclen;
> memcpy(dest, src, cplen);
> ! dest[cplen] = '\0';
> ! return cplen;
> case SYBBINARY:
> cplen = srclen > destlen ? destlen : srclen;
> memcpy(dest, src, cplen);




Archive powered by MHonArc 2.6.24.

Top of Page