FreeTDS Development Group

[Steve Langasek <vorlon AT netexpress.net>]

Hi Brian,

On Wed, 24 Jan 2001, Brian Bruns wrote:

> Technically it is defined by the API as length of dest, I don't have the
> doc in front of me but I believe it is something like 'size of buffer
> into which the data will be copied' (remember that tds_convert is called
> by dbconvert, ct_convert and implicitly by binding a to a different type).

> So there is an off by one error, and I'll apply the patch.

At the bottom is a patch that addresses all of the off-by-one errors I found
in tds/convert.c.  In addition, it ensures that all SYBTEXT and SYBCHAR
targets are null-terminated strings, which if I understand correctly is the
proper behavior.

I also noticed these lines during the process:

TDS_INT tds_convert_money(int srctype,unsigned char *src,
        int desttype,unsigned char *dest,TDS_INT destlen)
{
long high;
unsigned long low;
double dmoney;

...

                case SYBFLT8:
                        /* Used memcpy to avoid alignment/bus errors */
                        memcpy(&high, src, 4);
                        memcpy(&low, src+4, 4);
                        dmoney = (double)high * 65536 * 65536 + (double)low;
                        dmoney = dmoney / 10000;
                        *(double *)dest = dmoney;
                        break;  

I don't have a patch for this yet, but just as a heads-up, this behavior is
not endian-clean on machines with a 64-bit word.  Since a long int is 8 bytes
long on alpha, ppc, etc., these memcpy's only write to the first 4 bytes of
the long... and those first 4 bytes aren't guaranteed to be the low bytes
we're looking for.

If anyone here is better at writing endian-clean code than I am, you're
welcome to beat me to patching this. :)

Regards,
Steve Langasek
postmodern programmer

Index: src/tds/convert.c
===================================================================
RCS file: /Repository/freetds/src/tds/convert.c,v
retrieving revision 1.45
diff -u -w -r1.45 convert.c
--- convert.c   2001/01/21 20:47:11     1.45
+++ convert.c   2001/01/25 20:21:03
@@ -97,13 +97,13 @@
       case SYBTEXT:
         cplen = srclen > destlen ? destlen : srclen;
          memcpy(dest, src, cplen);
+         dest[cplen-1] = '\0';
          return cplen;
       case SYBCHAR:
-        cplen = srclen;
-        if (destlen<cplen) cplen = destlen;
+        cplen = srclen > destlen ? destlen : srclen;
         if (cplen>255) cplen=255;
          memcpy(dest, src, cplen);
-        dest[cplen]='\0';
+        dest[cplen-1]='\0';
          return cplen;
    }
    return 0;
@@ -126,7 +126,7 @@
        * XXX Case for -1 in switch statement because upper levels don't
        *     have a way to bind to wide-character types.
        */
-      TDS_UINT i, cplen, char_limit = 255;
+      TDS_UINT i, cplen, char_limit = 256;
       utf16_t* wsrc  = (utf16_t*)src;
       utf16_t* wdest = (utf16_t*)dest;
       assert(sizeof(utf16_t) == 2);
@@ -155,10 +155,7 @@
                       cplen = srclen > destlen ? destlen : srclen;
                       for (i = 0; i < cplen; ++i)
                               dest[i] = (unsigned char)wsrc[i];
-                      if (destlen < srclen)
-                              dest[destlen - 1] = 0;
-                      else
-                              dest[cplen] = 0;
+                      dest[cplen-1] = 0;
                       return strlen(dest);
       }
        return 0;
@@ -176,7 +173,7 @@
       case SYBCHAR:
       case SYBVARCHAR:
          /* FIX ME */
-        if (destlen < 2) return 0;
+        if (destlen < 3) return 0;
 
         d=0;
         dest[d++]='0';
@@ -588,9 +585,10 @@
       case SYBVARCHAR:
       /* A real type has a 7 digit precision plus a two digit exponent
        *   "-d.dddddde+dd"
-       * so it should have >=13 bytes allocated, right?
+       * so it should have >=14 bytes allocated (one for the terminating
+       * NULL), right?
        */
-         if (destlen >= 0 && destlen < 13)  return TDS_FAIL;
+         if (destlen >= 0 && destlen < 14)  return TDS_FAIL;
          sprintf(dest,"%.7g", the_value);
          return strlen(dest);
       case SYBFLT8:
@@ -614,9 +612,10 @@
       case SYBVARCHAR:
       /* A flt8 type has a 15 digit precision plus a three digit exponent
        *   "-d.dddddddddddddde+ddd"
-       * so it should have >=22 bytes allocated, right?
+       * so it should have >=23 bytes allocated (one for the terminating
+       * NULL), right?
        */
-         if (destlen >= 0 && destlen < 22)  return TDS_FAIL;
+         if (destlen >= 0 && destlen < 23)  return TDS_FAIL;
          sprintf(dest,"%.15g", the_value);
          return strlen(dest);
       case SYBREAL: