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(at)candle(dot)pha(dot)pa(dot)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)