Skip to content

Commit a1212d0

Browse files
tglsfdctglsfdc
committed
Fix various confusions of pointers and OIDs, unsafe assumptions about nulls,
etc. I think this will fix the current buildfarm issues ...
1 parent cef8efc commit a1212d0

File tree

1 file changed

+32
-17
lines changed

1 file changed

+32
-17
lines changed

‎src/backend/commands/foreigncmds.c

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/commands/foreigncmds.c,v 1.2 2008/12/20 09:40:56 heikki Exp $
10+
* $PostgreSQL: pgsql/src/backend/commands/foreigncmds.c,v 1.3 2008/12/20 15:51:28 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -35,6 +35,8 @@
3535
/*
3636
* Convert a DefElem list to the text array format that is used in
3737
* pg_foreign_data_wrapper, pg_foreign_server, and pg_user_mapping.
38+
* Returns the array in the form of a Datum, or PointerGetDatum(NULL)
39+
* if the list is empty.
3840
*
3941
* Note: The array is usually stored to database without further
4042
* processing, hence any validation should be done before this
@@ -45,13 +47,13 @@ optionListToArray(List *options)
4547
{
4648
ArrayBuildState *astate = NULL;
4749
ListCell *cell;
48-
text *t;
4950

50-
foreach (cell, options)
51+
foreach(cell, options)
5152
{
5253
DefElem *def = lfirst(cell);
53-
const char *value = "";
54+
const char *value;
5455
Size len;
56+
text *t;
5557

5658
value = defGetString(def);
5759
len = VARHDRSZ + strlen(def->defname) + 1 + strlen(value);
@@ -76,6 +78,9 @@ optionListToArray(List *options)
7678
* The result is converted to array of text suitable for storing in
7779
* options.
7880
*
81+
* Returns the array in the form of a Datum, or PointerGetDatum(NULL)
82+
* if the list is empty.
83+
*
7984
* This is used by CREATE/ALTER of FOREIGN DATA WRAPPER/SERVER/USER
8085
* MAPPING. In the ALTER cases, oldOptions is the current text array
8186
* of options.
@@ -90,15 +95,15 @@ transformGenericOptions(Datum oldOptions,
9095
List *resultOptions = untransformRelOptions(oldOptions);
9196
ListCell *optcell;
9297

93-
foreach (optcell, optionDefList)
98+
foreach(optcell, optionDefList)
9499
{
100+
OptionDefElem *od = lfirst(optcell);
95101
ListCell *cell;
96102
ListCell *prev = NULL;
97-
OptionDefElem *od = lfirst(optcell);
98103

99104
/*
100105
* Find the element in resultOptions. We need this for
101-
* validation in all cases.
106+
* validation in all cases. Also identify the previous element.
102107
*/
103108
foreach (cell, resultOptions)
104109
{
@@ -190,7 +195,6 @@ AlterForeignDataWrapperOwner(const char *name, Oid newOwnerId)
190195
HeapTuple tup;
191196
Relation rel;
192197
Oid fdwId;
193-
194198
Form_pg_foreign_data_wrapper form;
195199

196200
/* Must be a superuser to change a FDW owner */
@@ -345,7 +349,8 @@ CreateForeignDataWrapper(CreateFdwStmt *stmt)
345349
*/
346350
rel = heap_open(ForeignDataWrapperRelationId, RowExclusiveLock);
347351

348-
MemSet(nulls, false, Natts_pg_foreign_data_wrapper);
352+
memset(values, 0, sizeof(values));
353+
memset(nulls, false, sizeof(nulls));
349354

350355
values[Anum_pg_foreign_data_wrapper_fdwname - 1] =
351356
DirectFunctionCall1(namein, CStringGetDatum(stmt->fdwname));
@@ -359,7 +364,8 @@ CreateForeignDataWrapper(CreateFdwStmt *stmt)
359364
*/
360365
fdwlib = GetForeignDataWrapperLibrary(stmt->library);
361366

362-
fdwoptions = transformGenericOptions(0, stmt->options, FdwOpt, NULL,
367+
fdwoptions = transformGenericOptions(PointerGetDatum(NULL), stmt->options,
368+
FdwOpt, NULL,
363369
fdwlib->validateOptionList);
364370

365371
if (PointerIsValid(DatumGetPointer(fdwoptions)))
@@ -447,6 +453,7 @@ AlterForeignDataWrapper(AlterFdwStmt *stmt)
447453
tp,
448454
Anum_pg_foreign_data_wrapper_fdwlibrary,
449455
&isnull);
456+
Assert(!isnull);
450457
fdwlib = GetForeignDataWrapperLibrary(TextDatumGetCString(datum));
451458
}
452459

@@ -460,13 +467,15 @@ AlterForeignDataWrapper(AlterFdwStmt *stmt)
460467
tp,
461468
Anum_pg_foreign_data_wrapper_fdwoptions,
462469
&isnull);
470+
if (isnull)
471+
datum = PointerGetDatum(NULL);
463472

464473
/* Transform the options */
465474
datum = transformGenericOptions(datum, stmt->options, FdwOpt,
466475
NULL, fdwlib->validateOptionList);
467476

468477
if (PointerIsValid(DatumGetPointer(datum)))
469-
repl_val[Anum_pg_foreign_data_wrapper_fdwoptions - 1] = ObjectIdGetDatum(datum);
478+
repl_val[Anum_pg_foreign_data_wrapper_fdwoptions - 1] = datum;
470479
else
471480
repl_null[Anum_pg_foreign_data_wrapper_fdwoptions - 1] = true;
472481

@@ -603,7 +612,8 @@ CreateForeignServer(CreateForeignServerStmt *stmt)
603612
*/
604613
rel = heap_open(ForeignServerRelationId, RowExclusiveLock);
605614

606-
MemSet(nulls, false, Natts_pg_foreign_server);
615+
memset(values, 0, sizeof(values));
616+
memset(nulls, false, sizeof(nulls));
607617

608618
values[Anum_pg_foreign_server_srvname - 1] =
609619
DirectFunctionCall1(namein, CStringGetDatum(stmt->servername));
@@ -628,7 +638,8 @@ CreateForeignServer(CreateForeignServerStmt *stmt)
628638
nulls[Anum_pg_foreign_server_srvacl - 1] = true;
629639

630640
/* Add server options */
631-
srvoptions = transformGenericOptions(0, stmt->options, ServerOpt, fdw,
641+
srvoptions = transformGenericOptions(PointerGetDatum(NULL), stmt->options,
642+
ServerOpt, fdw,
632643
fdw->lib->validateOptionList);
633644

634645
if (PointerIsValid(DatumGetPointer(srvoptions)))
@@ -722,6 +733,8 @@ AlterForeignServer(AlterForeignServerStmt *stmt)
722733
tp,
723734
Anum_pg_foreign_server_srvoptions,
724735
&isnull);
736+
if (isnull)
737+
datum = PointerGetDatum(NULL);
725738

726739
/* Prepare the options array */
727740
datum = transformGenericOptions(datum, stmt->options, ServerOpt,
@@ -868,13 +881,15 @@ CreateUserMapping(CreateUserMappingStmt *stmt)
868881
*/
869882
rel = heap_open(UserMappingRelationId, RowExclusiveLock);
870883

871-
MemSet(nulls, false, Natts_pg_user_mapping);
884+
memset(values, 0, sizeof(values));
885+
memset(nulls, false, sizeof(nulls));
872886

873887
values[Anum_pg_user_mapping_umuser - 1] = ObjectIdGetDatum(useId);
874888
values[Anum_pg_user_mapping_umserver - 1] = ObjectIdGetDatum(srv->serverid);
875889

876890
/* Add user options */
877-
useoptions = transformGenericOptions(0, stmt->options, UserMappingOpt,
891+
useoptions = transformGenericOptions(PointerGetDatum(NULL), stmt->options,
892+
UserMappingOpt,
878893
fdw, fdw->lib->validateOptionList);
879894

880895
if (PointerIsValid(DatumGetPointer(useoptions)))
@@ -931,12 +946,10 @@ AlterUserMapping(AlterUserMappingStmt *stmt)
931946
ObjectIdGetDatum(srv->serverid),
932947
0, 0);
933948
if (!OidIsValid(umId))
934-
{
935949
ereport(ERROR,
936950
(errcode(ERRCODE_UNDEFINED_OBJECT),
937951
errmsg("user mapping \"%s\" does not exist for the server",
938952
MappingUserName(useId))));
939-
}
940953

941954
/*
942955
* Must be owner of the server to alter user mapping.
@@ -972,6 +985,8 @@ AlterUserMapping(AlterUserMappingStmt *stmt)
972985
tp,
973986
Anum_pg_user_mapping_umoptions,
974987
&isnull);
988+
if (isnull)
989+
datum = PointerGetDatum(NULL);
975990

976991
/* Prepare the options array */
977992
datum = transformGenericOptions(datum, stmt->options, UserMappingOpt,

0 commit comments

Comments
 (0)