From 84b6d5f35941a0406210e7938d10c3cce4e11340 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Sun, 10 Jan 2010 04:26:36 +0000 Subject: [PATCH] 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. --- src/backend/access/common/heaptuple.c | 74 ++++++-------------------- src/backend/access/common/indextuple.c | 39 ++------------ src/backend/access/heap/heapam.c | 10 ++-- src/include/access/htup.h | 14 ++--- src/include/access/itup.h | 8 +-- 5 files changed, 36 insertions(+), 109 deletions(-) diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c index 1ece4f02bf..8d770a368b 100644 --- a/src/backend/access/common/heaptuple.c +++ b/src/backend/access/common/heaptuple.c @@ -50,7 +50,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/common/heaptuple.c,v 1.129 2010/01/02 16:57:33 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/common/heaptuple.c,v 1.130 2010/01/10 04:26:36 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -323,8 +323,7 @@ heap_attisnull(HeapTuple tup, int attnum) Datum nocachegetattr(HeapTuple tuple, int attnum, - TupleDesc tupleDesc, - bool *isnull) + TupleDesc tupleDesc) { HeapTupleHeader tup = tuple->t_data; Form_pg_attribute *att = tupleDesc->attrs; @@ -333,8 +332,6 @@ nocachegetattr(HeapTuple tuple, bool slow = false; /* do we have to walk attrs? */ int off; /* current offset within data */ - (void) isnull; /* not used */ - /* ---------------- * Three cases: * @@ -344,68 +341,32 @@ nocachegetattr(HeapTuple tuple, * ---------------- */ -#ifdef IN_MACRO -/* This is handled in the macro */ - Assert(attnum > 0); - - if (isnull) - *isnull = false; -#endif - attnum--; - if (HeapTupleNoNulls(tuple)) - { -#ifdef IN_MACRO -/* This is handled in the macro */ - if (att[attnum]->attcacheoff >= 0) - { - return fetchatt(att[attnum], - (char *) tup + tup->t_hoff + - att[attnum]->attcacheoff); - } -#endif - } - else + if (!HeapTupleNoNulls(tuple)) { /* * there's a null somewhere in the tuple * - * check to see if desired att is null + * check to see if any preceding bits are null... */ + int byte = attnum >> 3; + int finalbit = attnum & 0x07; -#ifdef IN_MACRO -/* This is handled in the macro */ - if (att_isnull(attnum, bp)) + /* check for nulls "before" final bit of last byte */ + if ((~bp[byte]) & ((1 << finalbit) - 1)) + slow = true; + else { - if (isnull) - *isnull = true; - return (Datum) NULL; - } -#endif + /* check for nulls in any "earlier" bytes */ + int i; - /* - * Now check to see if any preceding bits are null... - */ - { - int byte = attnum >> 3; - int finalbit = attnum & 0x07; - - /* check for nulls "before" final bit of last byte */ - if ((~bp[byte]) & ((1 << finalbit) - 1)) - slow = true; - else + for (i = 0; i < byte; i++) { - /* check for nulls in any "earlier" bytes */ - int i; - - for (i = 0; i < byte; i++) + if (bp[i] != 0xFF) { - if (bp[i] != 0xFF) - { - slow = true; - break; - } + slow = true; + break; } } } @@ -567,8 +528,7 @@ heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, bool *isnull) Assert(tup); /* Currently, no sys attribute ever reads as NULL. */ - if (isnull) - *isnull = false; + *isnull = false; switch (attnum) { diff --git a/src/backend/access/common/indextuple.c b/src/backend/access/common/indextuple.c index 61d7f61997..3ce377b854 100644 --- a/src/backend/access/common/indextuple.c +++ b/src/backend/access/common/indextuple.c @@ -9,7 +9,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/common/indextuple.c,v 1.90 2010/01/02 16:57:33 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/common/indextuple.c,v 1.91 2010/01/10 04:26:36 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -200,8 +200,7 @@ index_form_tuple(TupleDesc tupleDescriptor, Datum nocache_index_getattr(IndexTuple tup, int attnum, - TupleDesc tupleDesc, - bool *isnull) + TupleDesc tupleDesc) { Form_pg_attribute *att = tupleDesc->attrs; char *tp; /* ptr to data part of tuple */ @@ -210,8 +209,6 @@ nocache_index_getattr(IndexTuple tup, int data_off; /* tuple data offset */ int off; /* current offset within data */ - (void) isnull; /* not used */ - /* ---------------- * Three cases: * @@ -221,31 +218,11 @@ nocache_index_getattr(IndexTuple tup, * ---------------- */ -#ifdef IN_MACRO -/* This is handled in the macro */ - Assert(PointerIsValid(isnull)); - Assert(attnum > 0); - - *isnull = false; -#endif - data_off = IndexInfoFindDataOffset(tup->t_info); attnum--; - if (!IndexTupleHasNulls(tup)) - { -#ifdef IN_MACRO -/* This is handled in the macro */ - if (att[attnum]->attcacheoff >= 0) - { - return fetchatt(att[attnum], - (char *) tup + data_off + - att[attnum]->attcacheoff); - } -#endif - } - else + if (IndexTupleHasNulls(tup)) { /* * there's a null somewhere in the tuple @@ -256,16 +233,6 @@ nocache_index_getattr(IndexTuple tup, /* XXX "knows" t_bits are just after fixed tuple header! */ bp = (bits8 *) ((char *) tup + sizeof(IndexTupleData)); -#ifdef IN_MACRO -/* This is handled in the macro */ - - if (att_isnull(attnum, bp)) - { - *isnull = true; - return (Datum) NULL; - } -#endif - /* * Now check to see if any preceding bits are null... */ diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index a787dc43da..de061279a1 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.280 2010/01/02 16:57:34 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.281 2010/01/10 04:26:36 rhaas Exp $ * * * INTERFACE ROUTINES @@ -834,7 +834,7 @@ fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, return ( (attnum) > 0 ? ( - ((isnull) ? (*(isnull) = false) : (dummyret) NULL), + (*(isnull) = false), HeapTupleNoNulls(tup) ? ( (tupleDesc)->attrs[(attnum) - 1]->attcacheoff >= 0 ? @@ -844,18 +844,18 @@ fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, (tupleDesc)->attrs[(attnum) - 1]->attcacheoff) ) : - nocachegetattr((tup), (attnum), (tupleDesc), (isnull)) + nocachegetattr((tup), (attnum), (tupleDesc)) ) : ( att_isnull((attnum) - 1, (tup)->t_data->t_bits) ? ( - ((isnull) ? (*(isnull) = true) : (dummyret) NULL), + (*(isnull) = true), (Datum) NULL ) : ( - nocachegetattr((tup), (attnum), (tupleDesc), (isnull)) + nocachegetattr((tup), (attnum), (tupleDesc)) ) ) ) diff --git a/src/include/access/htup.h b/src/include/access/htup.h index 1982ea4da8..e5478a21b6 100644 --- a/src/include/access/htup.h +++ b/src/include/access/htup.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/access/htup.h,v 1.109 2010/01/02 16:58:00 momjian Exp $ + * $PostgreSQL: pgsql/src/include/access/htup.h,v 1.110 2010/01/10 04:26:36 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -763,7 +763,7 @@ extern void HeapTupleHeaderAdjustCmax(HeapTupleHeader tup, #define fastgetattr(tup, attnum, tupleDesc, isnull) \ ( \ AssertMacro((attnum) > 0), \ - (((isnull) != NULL) ? (*(isnull) = false) : (dummyret)NULL), \ + (*(isnull) = false), \ HeapTupleNoNulls(tup) ? \ ( \ (tupleDesc)->attrs[(attnum)-1]->attcacheoff >= 0 ? \ @@ -773,18 +773,18 @@ extern void HeapTupleHeaderAdjustCmax(HeapTupleHeader tup, (tupleDesc)->attrs[(attnum)-1]->attcacheoff) \ ) \ : \ - nocachegetattr((tup), (attnum), (tupleDesc), (isnull)) \ + nocachegetattr((tup), (attnum), (tupleDesc)) \ ) \ : \ ( \ att_isnull((attnum)-1, (tup)->t_data->t_bits) ? \ ( \ - (((isnull) != NULL) ? (*(isnull) = true) : (dummyret)NULL), \ + (*(isnull) = true), \ (Datum)NULL \ ) \ : \ ( \ - nocachegetattr((tup), (attnum), (tupleDesc), (isnull)) \ + nocachegetattr((tup), (attnum), (tupleDesc)) \ ) \ ) \ ) @@ -818,7 +818,7 @@ extern Datum fastgetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, ( \ ((attnum) > (int) HeapTupleHeaderGetNatts((tup)->t_data)) ? \ ( \ - (((isnull) != NULL) ? (*(isnull) = true) : (dummyret)NULL), \ + (*(isnull) = true), \ (Datum)NULL \ ) \ : \ @@ -838,7 +838,7 @@ extern void heap_fill_tuple(TupleDesc tupleDesc, uint16 *infomask, bits8 *bit); extern bool heap_attisnull(HeapTuple tup, int attnum); extern Datum nocachegetattr(HeapTuple tup, int attnum, - TupleDesc att, bool *isnull); + TupleDesc att); extern Datum heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, bool *isnull); extern HeapTuple heap_copytuple(HeapTuple tuple); diff --git a/src/include/access/itup.h b/src/include/access/itup.h index fea09c4880..a2522337d7 100644 --- a/src/include/access/itup.h +++ b/src/include/access/itup.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/access/itup.h,v 1.53 2010/01/02 16:58:00 momjian Exp $ + * $PostgreSQL: pgsql/src/include/access/itup.h,v 1.54 2010/01/10 04:26:36 rhaas Exp $ * *------------------------------------------------------------------------- */ @@ -110,7 +110,7 @@ typedef IndexAttributeBitMapData *IndexAttributeBitMap; + (tupleDesc)->attrs[(attnum)-1]->attcacheoff) \ ) \ : \ - nocache_index_getattr((tup), (attnum), (tupleDesc), (isnull)) \ + nocache_index_getattr((tup), (attnum), (tupleDesc)) \ ) \ : \ ( \ @@ -121,7 +121,7 @@ typedef IndexAttributeBitMapData *IndexAttributeBitMap; ) \ : \ ( \ - nocache_index_getattr((tup), (attnum), (tupleDesc), (isnull)) \ + nocache_index_getattr((tup), (attnum), (tupleDesc)) \ ) \ ) \ ) @@ -142,7 +142,7 @@ typedef IndexAttributeBitMapData *IndexAttributeBitMap; extern IndexTuple index_form_tuple(TupleDesc tupleDescriptor, Datum *values, bool *isnull); extern Datum nocache_index_getattr(IndexTuple tup, int attnum, - TupleDesc tupleDesc, bool *isnull); + TupleDesc tupleDesc); extern void index_deform_tuple(IndexTuple tup, TupleDesc tupleDescriptor, Datum *values, bool *isnull); extern IndexTuple CopyIndexTuple(IndexTuple source);