Re: BUG #5237: strange int->bit and bit->int conversions - Mailing list pgsql-hackers

The bitfromint8() and bitfromint4() are hosed. They produce wrong
results when the BIT size is more than 64 and 32 respectively, and the
BIT size is not multiple of 8, and the most significant byte of the
integer value is not 0x00 or 0xff.

For example:

test=# select (11::int4<<23 | 11::int4)::bit(32);
  00000101100000000000000000001011

test=# select (11::int4<<23 | 11::int4)::bit(33);
  000001011100000000000000000001011

test=# select (11::int4<<23 | 11::int4)::bit(39);
  000001010000101100000000000000000001011

test=# select (11::int4<<23 | 11::int4)::bit(40);
  0000000000000101100000000000000000001011

The ::bit(33) and ::bit(39) conversions are wrong.

The patch re-implements the conversion functions.
diff -urp -x '*.o' -x '*.txt' -x '*.so' postgresql-8.4.1-orig/src/backend/utils/adt/varbit.c
postgresql-8.4.1/src/backend/utils/adt/varbit.c
--- postgresql-8.4.1-orig/src/backend/utils/adt/varbit.c    2009-12-12 09:19:13.000000000 -0600
+++ postgresql-8.4.1/src/backend/utils/adt/varbit.c    2009-12-12 10:29:59.000000000 -0600
@@ -1321,8 +1321,8 @@ bitfromint4(PG_FUNCTION_ARGS)
     VarBit       *result;
     bits8       *r;
     int            rlen;
-    int            destbitsleft,
-                srcbitsleft;
+    int const    srcbits=sizeof(a)*BITS_PER_BYTE;
+    int i;

     if (typmod <= 0)
         typmod = 1;                /* default bit length */
@@ -1333,32 +1333,21 @@ bitfromint4(PG_FUNCTION_ARGS)
     VARBITLEN(result) = typmod;

     r = VARBITS(result);
-    destbitsleft = typmod;
-    srcbitsleft = 32;
-    /* drop any input bits that don't fit */
-    srcbitsleft = Min(srcbitsleft, destbitsleft);
-    /* sign-fill any excess bytes in output */
-    while (destbitsleft >= srcbitsleft + 8)
-    {
-        *r++ = (bits8) ((a < 0) ? BITMASK : 0);
-        destbitsleft -= 8;
-    }
-    /* store first fractional byte */
-    if (destbitsleft > srcbitsleft)
-    {
-        *r++ = (bits8) ((a >> (srcbitsleft - 8)) & BITMASK);
-        destbitsleft -= 8;
-    }
-    /* Now srcbitsleft and destbitsleft are the same, need not track both */
-    /* store whole bytes */
-    while (destbitsleft >= 8)
-    {
-        *r++ = (bits8) ((a >> (destbitsleft - 8)) & BITMASK);
-        destbitsleft -= 8;
+    rlen=(typmod+BITS_PER_BYTE-1)/BITS_PER_BYTE;
+
+    if (srcbits>=typmod) {
+        a<<=srcbits-typmod;
+        for (i=0; i!=rlen; ++i, a<<=BITS_PER_BYTE) r[i]=a>>(srcbits-BITS_PER_BYTE);
+    } else {
+        int sh=typmod%BITS_PER_BYTE;
+        int32 h=a>>sh;
+        int32 l=a<<(srcbits-sh);
+        size_t const zsize=rlen-sizeof(a)-(sh!=0);
+        bits8 sx=(a>=0)-1;
+        memset(r,sx,zsize);
+        for (i=0; i!=sizeof(a); ++i, h<<=8) r[zsize+i]=h>>(srcbits-BITS_PER_BYTE);
+        if (sh!=0) r[zsize+sizeof(a)]=l>>(srcbits-BITS_PER_BYTE);
     }
-    /* store last fractional byte */
-    if (destbitsleft > 0)
-        *r = (bits8) ((a << (8 - destbitsleft)) & BITMASK);

     PG_RETURN_VARBIT_P(result);
 }
@@ -1396,8 +1385,8 @@ bitfromint8(PG_FUNCTION_ARGS)
     VarBit       *result;
     bits8       *r;
     int            rlen;
-    int            destbitsleft,
-                srcbitsleft;
+    int const    srcbits=sizeof(a)*BITS_PER_BYTE;
+    int            i;

     if (typmod <= 0)
         typmod = 1;                /* default bit length */
@@ -1408,36 +1397,21 @@ bitfromint8(PG_FUNCTION_ARGS)
     VARBITLEN(result) = typmod;

     r = VARBITS(result);
-    destbitsleft = typmod;
-#ifndef INT64_IS_BUSTED
-    srcbitsleft = 64;
-#else
-    srcbitsleft = 32;            /* don't try to shift more than 32 */
-#endif
-    /* drop any input bits that don't fit */
-    srcbitsleft = Min(srcbitsleft, destbitsleft);
-    /* sign-fill any excess bytes in output */
-    while (destbitsleft >= srcbitsleft + 8)
-    {
-        *r++ = (bits8) ((a < 0) ? BITMASK : 0);
-        destbitsleft -= 8;
-    }
-    /* store first fractional byte */
-    if (destbitsleft > srcbitsleft)
-    {
-        *r++ = (bits8) ((a >> (srcbitsleft - 8)) & BITMASK);
-        destbitsleft -= 8;
-    }
-    /* Now srcbitsleft and destbitsleft are the same, need not track both */
-    /* store whole bytes */
-    while (destbitsleft >= 8)
-    {
-        *r++ = (bits8) ((a >> (destbitsleft - 8)) & BITMASK);
-        destbitsleft -= 8;
+    rlen=(typmod+BITS_PER_BYTE-1)/BITS_PER_BYTE;
+
+    if (srcbits>=typmod) {
+        a<<=srcbits-typmod;
+        for (i=0; i!=rlen; ++i, a<<=BITS_PER_BYTE) r[i]=a>>(srcbits-BITS_PER_BYTE);
+    } else {
+        int sh=typmod%BITS_PER_BYTE;
+        int64 h=a>>sh;
+        int64 l=a<<(srcbits-sh);
+        size_t const zsize=rlen-sizeof(a)-(sh!=0);
+        bits8 sx=(a>=0)-1;
+        memset(r,sx,zsize);
+        for (i=0; i!=sizeof(a); ++i, h<<=8) r[zsize+i]=h>>(srcbits-BITS_PER_BYTE);
+        if (sh!=0) r[zsize+sizeof(a)]=l>>(srcbits-BITS_PER_BYTE);
     }
-    /* store last fractional byte */
-    if (destbitsleft > 0)
-        *r = (bits8) ((a << (8 - destbitsleft)) & BITMASK);

     PG_RETURN_VARBIT_P(result);
 }