8000 Code review for array_fill patch: fix inadequate check for array size… · postgres/postgres@5618ece · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 5618ece

Browse files
tglsfdctglsfdc
committed
Code review for array_fill patch: fix inadequate check for array size overflow
and bogus documentation (dimension arrays are int[] not anyarray). Also the errhint() messages seem to be really errdetail(), since there is nothing heuristic about them. Some other trivial cosmetic improvements.
1 parent 673a30f commit 5618ece

File tree

3 files changed

+66
-64
lines changed

3 files changed

+66
-64
lines changed

‎doc/src/sgml/func.sgml

Lines changed: 8 additions & 9 deletions
< 8000 td data-grid-cell-id="diff-229a1fe1ecaae95e35dcb9270c20c6d0bf37f33fbf019dfbd7f9ef014b07c0e3-9196-9195-2" data-line-anchor="diff-229a1fe1ecaae95e35dcb9270c20c6d0bf37f33fbf019dfbd7f9ef014b07c0e3R9195" data-selected="false" role="gridcell" style="background-color:var(--bgColor-default);padding-right:24px" tabindex="-1" valign="top" class="focusable-grid-cell diff-text-cell right-side-diff-cell left-side">
</para>
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.442 2008/07/18 03:32:51 tgl Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.443 2008/07/21 04:47:00 tgl Exp $ -->
22

33
<chapter id="functions">
44
<title>Functions and Operators</title>
@@ -9186,17 +9186,16 @@ SELECT NULLIF(value, '(none)') ...
91869186
</sect2>
91879187
</sect1>
91889188

9189-
91909189
<sect1 id="functions-array">
91919190
<title>Array Functions and Operators</title>
91929191

91939192
<para>
91949193
<xref linkend="array-operators-table"> shows the operators
9195-
available for <type>array</type> types.
9194+
available for array types.
91969195
91979196

91989197
<table id="array-operators-table">
9199-
<title><type>array</type> Operators</title>
9198+
<title>Array Operators</title>
92009199
<tgroup cols="4">
92019200
<thead>
92029201
<row>
@@ -9326,7 +9325,7 @@ SELECT NULLIF(value, '(none)') ...
93269325
</para>
93279326

93289327
<table id="array-functions-table">
9329-
<title><type>array</type> Functions</title>
9328+
<title>Array Functions</title>
93309329
<tgroup cols="5">
93319330
<thead>
93329331
<row>
@@ -9374,13 +9373,13 @@ SELECT NULLIF(value, '(none)') ...
93749373
<row>
93759374
<entry>
93769375
<literal>
9377-
<function>array_fill</function>(<type>anyelement</type>, <type>anyarray</type>,
9378-
<optional>, <type>anyarray</type>< 8000 /optional>)
9376+
<function>array_fill</function>(<type>anyelement</type>, <type>int[]</type>,
9377+
<optional>, <type>int[]</type></optional>)
93799378
</literal>
93809379
</entry>
93819380
<entry><type>anyarray</type></entry>
9382-
<entry>returns an array initialized with supplied value,
9383-
dimensions, and lower bounds</entry>
9381+
<entry>returns an array initialized with supplied value and
9382+
dimensions, optionally with lower bounds other than 1</entry>
93849383
<entry><literal>array_fill(7, ARRAY[3], ARRAY[2])</literal></entry>
93859384
<entry><literal>[2:4]={7,7,7}</literal></entry>
93869385
</row>

‎src/backend/utils/adt/arrayfuncs.c

Lines changed: 57 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/utils/adt/arrayfuncs.c,v 1.146 2008/07/16 00:48:53 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/utils/adt/arrayfuncs.c,v 1.147 2008/07/21 04:47:00 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -97,9 +97,9 @@ static void array_insert_slice(ArrayType *destArray, ArrayType *origArray,
9797
static int array_cmp(FunctionCallInfo fcinfo);
9898
static ArrayType *create_array_envelope(int ndims, int *dimv, int *lbv, int nbytes,
9999
Oid elmtype, int dataoffset);
100-
static ArrayType *array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value,
101-
Oid elmtype, bool isnull,
102-
FunctionCallInfo fcinfo);
100+
static ArrayType *array_fill_internal(ArrayType *dims, ArrayType *lbs,
101+
Datum value, bool isnull, Oid elmtype,
102+
FunctionCallInfo fcinfo);
103103

104104

105105
/*
@@ -4245,7 +4245,7 @@ typedef struct generate_subscripts_fctx
42454245
bool reverse;
42464246
} generate_subscripts_fctx;
42474247

4248-
/*
4248+
/*
42494249
* generate_subscripts(array anyarray, dim int [, reverse bool])
42504250
* Returns all subscripts of the array for any dimension
42514251
*/
@@ -4335,7 +4335,7 @@ array_fill_with_lower_bounds(PG_FUNCTION_ARGS)
43354335
bool isnull;
43364336

43374337
if (PG_ARGISNULL(1) || PG_ARGISNULL(2))
4338-
ereport(ERROR,
4338+
ereport(ERROR,
43394339
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
43404340
errmsg("dimension array or low bound array cannot be NULL")));
43414341

@@ -4353,11 +4353,11 @@ array_fill_with_lower_bounds(PG_FUNCTION_ARGS)
43534353
isnull = true;
43544354
}
43554355

4356-
elmtype = get_fn_expr_argtype(fcinfo->flinfo, 0);
4357-
if (!OidIsValid(elmtype))
4358-
elog(ERROR, "could not determine data type of input");
4356+
elmtype = get_fn_expr_argtype(fcinfo->flinfo, 0);
4357+
if (!OidIsValid(elmtype))
4358+
elog(ERROR, "could not determine data type of input");
43594359

4360-
result = array_fill_internal(dims, lbs, value, elmtype, isnull, fcinfo);
4360+
result = array_fill_internal(dims, lbs, value, isnull, elmtype, fcinfo);
43614361
PG_RETURN_ARRAYTYPE_P(result);
43624362
}
43634363

@@ -4375,7 +4375,7 @@ array_fill(PG_FUNCTION_ARGS)
43754375
bool isnull;
43764376

43774377
if (PG_ARGISNULL(1))
4378-
ereport(ERROR,
4378+
ereport(ERROR,
43794379
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
43804380
errmsg("dimension array or low bound array cannot be NULL")));
43814381

@@ -4392,17 +4392,17 @@ array_fill(PG_FUNCTION_ARGS)
43924392
isnull = true;
43934393
}
43944394

4395-
elmtype = get_fn_expr_argtype(fcinfo->flinfo, 0);
4396-
if (!OidIsValid(elmtype))
4397-
elog(ERROR, "could not determine data type of input");
4395+
elmtype = get_fn_expr_argtype(fcinfo->flinfo, 0);
4396+
if (!OidIsValid(elmtype))
4397+
elog(ERROR, "could not determine data type of input");
43984398

4399-
result = array_fill_internal(dims, NULL, value, elmtype, isnull, fcinfo);
4399+
result = array_fill_internal(dims, NULL, value, isnull, elmtype, fcinfo);
44004400
PG_RETURN_ARRAYTYPE_P(result);
44014401
}
44024402

44034403
static ArrayType *
44044404
create_array_envelope(int ndims, int *dimv, int *lbsv, int nbytes,
4405-
Oid elmtype, int dataoffset)
4405+
Oid elmtype, int dataoffset)
44064406
{
44074407
ArrayType *result;
44084408

@@ -4418,44 +4418,44 @@ create_array_envelope(int ndims, int *dimv, int *lbsv, int nbytes,
44184418
}
44194419

44204420
static ArrayType *
4421-
array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value,
4422-
Oid elmtype, bool isnull,
4423-
FunctionCallInfo fcinfo)
4421+
array_fill_internal(ArrayType *dims, ArrayType *lbs,
4422+
Datum value, bool isnull, Oid elmtype,
4423+
FunctionCallInfo fcinfo)
44244424
{
44254425
ArrayType *result;
44264426
int *dimv;
44274427
int *lbsv;
44284428
int ndims;
44294429
int nitems;
44304430
int deflbs[MAXDIM];
4431-
int16 elmlen;
4432-
bool elmbyval;
4431+
int16 elmlen;
4432+
bool elmbyval;
44334433
char elmalign;
44344434
ArrayMetaState *my_extra;
44354435

4436-
/*
4436+
/*
44374437
* Params checks
44384438
*/
44394439
if (ARR_NDIM(dims) != 1)
44404440
ereport(ERROR,
44414441
(errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),
44424442
errmsg("wrong number of array subscripts"),
4443-
errhint("Dimension array must be one dimensional.")));
4443+
errdetail("Dimension array must be one dimensional.")));
44444444

44454445
if (ARR_LBOUND(dims)[0] != 1)
44464446
ereport(ERROR,
44474447
(errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),
44484448
errmsg("wrong range of array_subscripts"),
4449-
errhint("Lower bound of dimension array must be one.")));
4450-
4449+
errdetail("Lower bound of dimension array must be one.")));
4450+
44514451
if (ARR_HASNULL(dims))
4452-
ereport(ERROR,
4452+
ereport(ERROR,
44534453
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
44544454
errmsg("dimension values cannot be null")));
44554455

44564456
dimv = (int *) ARR_DATA_PTR(dims);
44574457
ndims = ARR_DIMS(dims)[0];
4458-
4458+
44594459
if (ndims < 0) /* we do allow zero-dimension arrays */
44604460
ereport(ERROR,
44614461
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -4465,38 +4465,38 @@ array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value,
44654465
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
44664466
errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
44674467
ndims, MAXDIM)));
4468-
4468+
44694469
if (lbs != NULL)
44704470
{
44714471
if (ARR_NDIM(lbs) != 1)
44724472
ereport(ERROR,
44734473
(errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),
44744474
errmsg("wrong number of array subscripts"),
4475-
errhint("Dimension array must be one dimensional.")));
4475+
errdetail("Dimension array must be one dimensional.")));
44764476

44774477
if (ARR_LBOUND(lbs)[0] != 1)
44784478
ereport(ERROR,
44794479
(errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),
44804480
errmsg("wrong range of array_subscripts"),
4481-
errhint("Lower bound of dimension array must be one.")));
4482-
4481+
errdetail("Lower bound of dimension array must be one.")));
4482+
44834483
if (ARR_HASNULL(lbs))
4484-
ereport(ERROR,
4484+
ereport(ERROR,
44854485
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
44864486
errmsg("dimension values cannot be null")));
44874487

44884488
if (ARR_DIMS(lbs)[0] != ndims)
44894489
ereport(ERROR,
44904490
(errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),
44914491
errmsg("wrong number of array_subscripts"),
4492-
errhint("Low bound array has different size than dimensions array.")));
4493-
4492+
errdetail("Low bound array has different size than dimensions array.")));
4493+
44944494
lbsv = (int *) ARR_DATA_PTR(lbs);
44954495
}
4496-
else
4496+
else
44974497
{
44984498
int i;
4499-
4499+
45004500
for (i = 0; i < MAXDIM; i++)
45014501
deflbs[i] = 1;
45024502

@@ -4506,9 +4506,8 @@ array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value,
45064506
/* fast track for empty array */
45074507
if (ndims == 0)
45084508
return construct_empty_array(elmtype);
4509-
4510-
nitems = ArrayGetNItems(ndims, dimv);
45114509

4510+
nitems = ArrayGetNItems(ndims, dimv);
45124511

45134512
/*
45144513
* We arrange to look up info about element type only once per series of
@@ -4543,48 +4542,52 @@ array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value,
45434542
int i;
45444543
char *p;
45454544
int nbytes;
4546-
Datum aux_value = value;
4545+
int totbytes;
45474546

45484547
/* make sure data is not toasted */
45494548
if (elmlen == -1)
45504549
value = PointerGetDatum(PG_DETOAST_DATUM(value));
45514550

45524551
nbytes = att_addlength_datum(0, elmlen, value);
45534552
nbytes = att_align_nominal(nbytes, elmalign);
4553+
Assert(nbytes > 0);
45544554

4555-
nbytes *= nitems;
4556-
/* check for overflow of total request */
4557-
if (!AllocSizeIsValid(nbytes))
4555+
totbytes = nbytes * nitems;
4556+
4557+
/* check for overflow of multiplication or total request */
4558+
if (totbytes / nbytes != nitems ||
4559+
!AllocSizeIsValid(totbytes))
45584560
ereport(ERROR,
45594561
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
45604562
errmsg("array size exceeds the maximum allowed (%d)",
45614563
(int) MaxAllocSize)));
45624564

4563-
nbytes += ARR_OVERHEAD_NONULLS(ndims);
4564-
result = create_array_envelope(ndims, dimv, lbsv, nbytes,
4565-
elmtype, 0);
4565+
/*
4566+
* This addition can't overflow, but it might cause us to go past
4567+
* MaxAllocSize. We leave it to palloc to complain in that case.
4568+
*/
4569+
totbytes += ARR_OVERHEAD_NONULLS(ndims);
4570+
4571+
result = create_array_envelope(ndims, dimv, lbsv, totbytes,
4572+
elmtype, 0);
4573+
45664574
p = ARR_DATA_PTR(result);
45674575
for (i = 0; i < nitems; i++)
45684576
p += ArrayCastAndSet(value, elmlen, elmbyval, elmalign, p);
4569-
4570-
/* cleaning up detoasted copies of datum */
4571-
if (aux_value != value)
4572-
pfree((Pointer) value);
45734577
}
45744578
else
45754579
{
45764580
int nbytes;
45774581
int dataoffset;
4578-
bits8 *bitmap;
45794582

45804583
dataoffset = ARR_OVERHEAD_WITHNULLS(ndims, nitems);
45814584
nbytes = dataoffset;
45824585

45834586
result = create_array_envelope(ndims, dimv, lbsv, nbytes,
4584-
elmtype, dataoffset);
4585-
bitmap = ARR_NULLBITMAP(result);
4586-
MemSet(bitmap, 0, (nitems + 7) / 8);
4587+
elmtype, dataoffset);
4588+
4589+
/* create_array_envelope already zeroed the bitmap, so we're done */
45874590
}
4588-
4591+
45894592
return result;
45904593
}

‎src/test/regress/expected/arrays.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -988,6 +988,6 @@ select array_fill(1, array[2,2], null);
988988
ERROR: dimension array or low bound array cannot be NULL
989989
select array_fill(1, array[3,3], array[1,1,1]);
990990
ERROR: wrong number of array_subscripts
991-
HINT: Low bound array has different size than dimensions array.
991+
DETAIL: Low bound array has different size than dimensions array.
992992
select array_fill(1, array[1,2,null]);
993993
ERROR: dimension values cannot be null

0 commit comments

Comments
 (0)
0