Re: [HACKERS] questionable code in heap_formtuple()

> around line 812 in access/common/heaptuple.c:
>
>     len = sizeof *tuple - sizeof tuple->t_bits;
>
> This seems questionable for me.

This is interesting.  They are getting the sizeof tuple->t_bits, not the
offset, so aren't they getting this very wrong?

They are computing the size of the tuple, minus the t_bits field, which
means nothing, no?

>
> tuple is a pointer to struct HeaptupleData.
>
> typedef struct HeapTupleData
> {
>     unsigned int t_len;            /* length of entire tuple */
>
>     [snip]
>
>     uint8        t_hoff;            /* sizeof tuple header */
>
>     bits8        t_bits[MinHeapTupleBitmapSize / 8];
>     /* bit map of domains */
>
>     /* MORE DATA FOLLOWS AT END OF STRUCT */
> } HeapTupleData;
>
> I think the code tries to calculate the offset from top of the
> structure to t_bits. t_bits is the last structure member of
> HeapTupleData, and that would give the offset...

Does it?

>
> No. since the size of the whole structure is aligned to 2-byte, there
> is a "padding" byte after t_bits.
>
> I think more acculate way to calculate the offset is:
>
>     len = (char *)&tuple->t_bits[0] - (char *)tuple;

Yours is much better.

>
> I ran a test and found the first one gives len = 36, while second one
> gives 35.
>
> I'm not sure how this affects. maybe nothing (len is aligned to 8-byte
> boundary later).

Should affect a lot, if I am understanding it properly.  This is also
done in heap_addheader() later in the file.

I just ran a little test:

    #include <stdio.h>

    struct test {
            int x;
            int y;
    } test;

    main()
    {
        printf("%d\n",sizeof(test.y));
        return 0;
    }

and with sizeof int == 4, the program returns 4, which is not the offset
of y, but the size of y.  6.3.2 has the same code.

I must be misunderstanding this.

--
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)