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)