Skip to content

Commit 84b6d5f

Browse files
robertmhaasrobertmhaas
committed
Remove partial, broken support for NULL pointers when fetching attributes.
Previously, fastgetattr() and heap_getattr() tested their fourth argument against a null pointer, but any attempt to use them with a literal-NULL fourth argument evaluated to *(void *)0, resulting in a compiler error. Remove these NULL tests to avoid leading future readers of this code to believe that this has a chance of working. Also clean up related legacy code in nocachegetattr(), heap_getsysattr(), and nocache_index_getattr(). The new coding standard is that any code which calls a getattr-type function or macro which takes an isnull argument MUST pass a valid boolean pointer. Per discussion with Bruce Momjian, Tom Lane, Alvaro Herrera.
1 parent 8b9fa7a commit 84b6d5f

File tree

5 files changed

+36
-109
lines changed

5 files changed

+36
-109
lines changed

‎src/backend/access/common/heaptuple.c

Lines changed: 17 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
*
5151
*
5252
* IDENTIFICATION
53-
* $PostgreSQL: pgsql/src/backend/access/common/heaptuple.c,v 1.129 2010/01/02 16:57:33 momjian Exp $
53+
* $PostgreSQL: pgsql/src/backend/access/common/heaptuple.c,v 1.130 2010/01/10 04:26:36 rhaas Exp $
5454
*
5555
*-------------------------------------------------------------------------
5656
*/
@@ -323,8 +323,7 @@ heap_attisnull(HeapTuple tup, int attnum)
323323
Datum
324324
nocachegetattr(HeapTuple tuple,
325325
int attnum,
326-
TupleDesc tupleDesc,
327-
bool *isnull)
326+
TupleDesc tupleDesc)
328327
{
329328
HeapTupleHeader tup = tuple->t_data;
330329
Form_pg_attribute *att = tupleDesc->attrs;
@@ -333,8 +332,6 @@ nocachegetattr(HeapTuple tuple,
333332
bool slow = false; /* do we have to walk attrs? */
334333
int off; /* current offset within data */
335334

336-
(void) isnull; /* not used */
337-
338335
/* ----------------
339336
* Three cases:
340337
*
@@ -344,68 +341,32 @@ nocachegetattr(HeapTuple tuple,
344341
* ----------------
345342
*/
346343

347-
#ifdef IN_MACRO
348-
/* This is handled in the macro */
349-
Assert(attnum > 0);
350-
351-
if (isnull)
352-
*isnull = false;
353-
#endif
354-
355344
attnum--;
356345

357-
if (HeapTupleNoNulls(tuple))
358-
{
359-
#ifdef IN_MACRO
360-
/* This is handled in the macro */
361-
if (att[attnum]->attcacheoff >= 0)
362-
{
363-
return fetchatt(att[attnum],
364-
(char *) tup + tup->t_hoff +
365-
att[attnum]->attcacheoff);
366-
}
367-
#endif
368-
}
369-
else
346+
if (!HeapTupleNoNulls(tuple))
370347
{
371348
/*
372349
* there's a null somewhere in the tuple
373350
*
374-
* check to see if desired att is null
351+
* check to see if any preceding bits are null...
375352
*/
353+
int byte = attnum >> 3;
354+
int finalbit = attnum & 0x07;
376355

377-
#ifdef IN_MACRO
378-
/* This is handled in the macro */
379-
if (att_isnull(attnum, bp))
380-
{
381-
if (isnull)
382-
*isnull = true;
383-
return (Datum) NULL;
384-
}
385-
#endif
386-
387-
/*
388-
* Now check to see if any preceding bits are null...
389-
*/
356+
/* check for nulls "before" final bit of last byte */
357+
if ((~bp[byte]) & ((1 << finalbit) - 1))
358+
slow = true;
359+
else
390360
{
391-
int byte = attnum >> 3;
392-
int finalbit = attnum & 0x07;
361+
/* check for nulls in any "earlier" bytes */
362+
int i;
393363

394-
/* check for nulls "before" final bit of last byte */
395-
if ((~bp[byte]) & ((1 << finalbit) - 1))
396-
slow = true;
397-
else
364+
for (i = 0; i < byte; i++)
398365
{
399-
/* check for nulls in any "earlier" bytes */
400-
int i;
401-
402-
for (i = 0; i < byte; i++)
366+
if (bp[i] != 0xFF)
403367
{
404-
if (bp[i] != 0xFF)
405-
{
406-
slow = true;
407-
break;
408-
}
368+
slow = true;
369+
break;
409370
}
410371
}
411372
}
@@ -567,8 +528,7 @@ heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, bool *isnull)
567528
Assert(tup);
568529

569530
/* Currently, no sys attribute ever reads as NULL. */
570-
if (isnull)
571-
*isnull = false;
531+
*isnull = false;
572532

573533
switch (attnum)
574534
{

‎src/backend/access/common/indextuple.c

Lines changed: 3 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/backend/access/common/indextuple.c,v 1.90 2010/01/02 16:57:33 momjian Exp $
12+
* $PostgreSQL: pgsql/src/backend/access/common/indextuple.c,v 1.91 2010/01/10 04:26:36 rhaas Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -200,8 +200,7 @@ index_form_tuple(TupleDesc tupleDescriptor,
200200
Datum
201201
nocache_index_getattr(IndexTuple tup,
202202
int attnum,
203-
TupleDesc tupleDesc,
204-
bool *isnull)
203+
TupleDesc tupleDesc)
205204
{
206205
Form_pg_attribute *att = tupleDesc->attrs;
207206
char *tp; /* ptr to data part of tuple */
@@ -210,8 +209,6 @@ nocache_index_getattr(IndexTuple tup,
210209
int data_off; /* tuple data offset */
211210
int off; /* current offset within data */
212211

213-
(void) isnull; /* not used */
214-
215212
/* ----------------
216213
* Three cases:
217214
*
@@ -221,31 +218,11 @@ nocache_index_getattr(IndexTuple tup,
221218
* ----------------
222219
*/
223220

224-
#ifdef IN_MACRO
225-
/* This is handled in the macro */
226-
Assert(PointerIsValid(isnull));
227-
Assert(attnum > 0);
228-
229-
*isnull = false;
230-
#endif
231-
232221
data_off = IndexInfoFindDataOffset(tup->t_info);
233222

234223
attnum--;
235224

236-
if (!IndexTupleHasNulls(tup))
237-
{
238-
#ifdef IN_MACRO
239-
/* This is handled in the macro */
240-
if (att[attnum]->attcacheoff >= 0)
241-
{
242-
return fetchatt(att[attnum],
243-
(char *) tup + data_off +
244-
att[attnum]->attcacheoff);
245-
}
246-
#endif
247-
}
248-
else
225+
if (IndexTupleHasNulls(tup))
249226
{
250227
/*
251228
* there's a null somewhere in the tuple
@@ -256,16 +233,6 @@ nocache_index_getattr(IndexTuple tup,
256233
/* XXX "knows" t_bits are just after fixed tuple header! */
257234
bp = (bits8 *) ((char *) tup + sizeof(IndexTupleData));
258235

259-
#ifdef IN_MACRO
260-
/* This is handled in the macro */
261-
262-
if (att_isnull(attnum, bp))
263-
{
264-
*isnull = true;
265-
return (Datum) NULL;
266-
}
267-
#endif
268-
269236
/*
270237
* Now check to see if any preceding bits are null...
271238
*/

‎src/backend/access/heap/heapam.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.280 2010/01/02 16:57:34 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.281 2010/01/10 04:26:36 rhaas Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -834,7 +834,7 @@ fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
834834
return (
835835
(attnum) > 0 ?
836836
(
837-
((isnull) ? (*(isnull) = false) : (dummyret) NULL),
837+
(*(isnull) = false),
838838
HeapTupleNoNulls(tup) ?
839839
(
840840
(tupleDesc)->attrs[(attnum) - 1]->attcacheoff >= 0 ?
@@ -844,18 +844,18 @@ fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
844844
(tupleDesc)->attrs[(attnum) - 1]->attcacheoff)
845845
)
846846
:
847-
nocachegetattr((tup), (attnum), (tupleDesc), (isnull))
847+
nocachegetattr((tup), (attnum), (tupleDesc))
848848
)
849849
:
850850
(
851851
att_isnull((attnum) - 1, (tup)->t_data->t_bits) ?
852852
(
853-
((isnull) ? (*(isnull) = true) : (dummyret) NULL),
853+
(*(isnull) = true),
854854
(Datum) NULL
855855
)
856856
:
857857
(
858-
nocachegetattr((tup), (attnum), (tupleDesc), (isnull))
858+
nocachegetattr((tup), (attnum), (tupleDesc))
859859
)
860860
)
861861
)

‎src/include/access/htup.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/access/htup.h,v 1.109 2010/01/02 16:58:00 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/access/htup.h,v 1.110 2010/01/10 04:26:36 rhaas Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -763,7 +763,7 @@ extern void HeapTupleHeaderAdjustCmax(HeapTupleHeader tup,
763763
#define fastgetattr(tup, attnum, tupleDesc, isnull) \
764764
( \
765765
AssertMacro((attnum) > 0), \
766-
(((isnull) != NULL) ? (*(isnull) = false) : (dummyret)NULL), \
766+
(*(isnull) = false), \
767767
HeapTupleNoNulls(tup) ? \
768768
( \
769769
(tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \
@@ -773,18 +773,18 @@ extern void HeapTupleHeaderAdjustCmax(HeapTupleHeader tup,
773773
(tupleDesc)->attrs[(attnum)-1]->attcacheoff) \
774774
) \
775775
: \
776-
nocachegetattr((tup), (attnum), (tupleDesc), (isnull)) \
776+
nocachegetattr((tup), (attnum), (tupleDesc)) \
777777
) \
778778
: \
779779
( \
780780
att_isnull((attnum)-1, (tup)->t_data->t_bits) ? \
781781
( \
782-
(((isnull) != NULL) ? (*(isnull) = true) : (dummyret)NULL), \
782+
(*(isnull) = true), \
783783
(Datum)NULL \
784784
) \
785785
: \
786786
( \
787-
nocachegetattr((tup), (attnum), (tupleDesc), (isnull)) \
787+
nocachegetattr((tup), (attnum), (tupleDesc)) \
788788
) \
789789
) \
790790
)
@@ -818,7 +818,7 @@ extern Datum fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
818818
( \
819819
((attnum) > (int) HeapTupleHeaderGetNatts((tup)->t_data)) ? \
820820
( \
821-
(((isnull) != NULL) ? (*(isnull) = true) : (dummyret)NULL), \
821+
(*(isnull) = true), \
822822
(Datum)NULL \
823823
) \
824824
: \
@@ -838,7 +838,7 @@ extern void heap_fill_tuple(TupleDesc tupleDesc,
838838
uint16 *infomask, bits8 *bit);
839839
extern bool heap_attisnull(HeapTuple tup, int attnum);
840840
extern Datum nocachegetattr(HeapTuple tup, int attnum,
841-
TupleDesc att, bool *isnull);
841+
TupleDesc att);
842842
extern Datum heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc,
843843
bool *isnull);
844844
extern HeapTuple heap_copytuple(HeapTuple tuple);

‎src/include/access/itup.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/access/itup.h,v 1.53 2010/01/02 16:58:00 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/access/itup.h,v 1.54 2010/01/10 04:26:36 rhaas Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -110,7 +110,7 @@ typedef IndexAttributeBitMapData *IndexAttributeBitMap;
110110
+ (tupleDesc)->attrs[(attnum)-1]->attcacheoff) \
111111
) \
112112
: \
113-
nocache_index_getattr((tup), (attnum), (tupleDesc), (isnull)) \
113+
nocache_index_getattr((tup), (attnum), (tupleDesc)) \
114114
) \
115115
: \
116116
( \
@@ -121,7 +121,7 @@ typedef IndexAttributeBitMapData *IndexAttributeBitMap;
121121
) \
122122
: \
123123
( \
124-
nocache_index_getattr((tup), (attnum), (tupleDesc), (isnull)) \
124+
nocache_index_getattr((tup), (attnum), (tupleDesc)) \
125125
) \
126126
) \
127127
)
@@ -142,7 +142,7 @@ typedef IndexAttributeBitMapData *IndexAttributeBitMap;
142142
extern IndexTuple index_form_tuple(TupleDesc tupleDescriptor,
143143
Datum *values, bool *isnull);
144144
extern Datum nocache_index_getattr(IndexTuple tup, int attnum,
145-
TupleDesc tupleDesc, bool *isnull);
145+
TupleDesc tupleDesc);
146146
extern void index_deform_tuple(IndexTuple tup, TupleDesc tupleDescriptor,
147147
Datum *values, bool *isnull);
148148
extern IndexTuple CopyIndexTuple(IndexTuple source);

0 commit comments

Comments
 (0)