On 08.08.2011 22:11, Alvaro Herrera wrote:
Perhaps the easiest way to fix it is as you suggest, by declaring the
struct to take a pointer rather than the value directly. Not sure how
to make both cases work sanely; the add_string_reloption path will need
updates.
Agreed, I propose the attached patch to do that. Are there any
extensions out there that use add_string_relopt(), that I could use for
testing?
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 4657425..900b222 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -371,8 +371,6 @@ allocate_reloption(bits32 kinds, int type, char *name, char *desc)
size_t size;
relopt_gen *newoption;
- Assert(type != RELOPT_TYPE_STRING);
-
oldcxt = MemoryContextSwitchTo(TopMemoryContext);
switch (type)
@@ -386,6 +384,9 @@ allocate_reloption(bits32 kinds, int type, char *name, char *desc)
case RELOPT_TYPE_REAL:
size = sizeof(relopt_real);
break;
+ case RELOPT_TYPE_STRING:
+ size = sizeof(relopt_string);
+ break;
default:
elog(ERROR, "unsupported option type");
return NULL; /* keep compiler quiet */
@@ -474,45 +475,29 @@ void
add_string_reloption(bits32 kinds, char *name, char *desc, char *default_val,
validate_string_relopt validator)
{
- MemoryContext oldcxt;
relopt_string *newoption;
- int default_len = 0;
-
- oldcxt = MemoryContextSwitchTo(TopMemoryContext);
-
- if (default_val)
- default_len = strlen(default_val);
- newoption = palloc0(sizeof(relopt_string) + default_len);
+ /* make sure the validator/default combination is sane */
+ if (validator)
+ (validator) (default_val);
- newoption->gen.name = pstrdup(name);
- if (desc)
- newoption->gen.desc = pstrdup(desc);
- else
- newoption->gen.desc = NULL;
- newoption->gen.kinds = kinds;
- newoption->gen.namelen = strlen(name);
- newoption->gen.type = RELOPT_TYPE_STRING;
+ newoption = (relopt_string *) allocate_reloption(kinds, RELOPT_TYPE_STRING,
+ name, desc);
newoption->validate_cb = validator;
if (default_val)
{
- strcpy(newoption->default_val, default_val);
- newoption->default_len = default_len;
+ newoption->default_val = MemoryContextStrdup(TopMemoryContext,
+ default_val);
+ newoption->default_len = strlen(default_val);
newoption->default_isnull = false;
}
else
{
- newoption->default_val[0] = '\0';
+ newoption->default_val = "";
newoption->default_len = 0;
newoption->default_isnull = true;
}
- /* make sure the validator/default combination is sane */
- if (newoption->validate_cb)
- (newoption->validate_cb) (newoption->default_val);
-
- MemoryContextSwitchTo(oldcxt);
-
add_reloption((relopt_gen *) newoption);
}
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index c7709cc..14f5034 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -108,7 +108,7 @@ typedef struct relopt_string
int default_len;
bool default_isnull;
validate_string_relopt validate_cb;
- char default_val[1]; /* variable length, zero-terminated */
+ char *default_val;
} relopt_string;
/* This is the table datatype for fillRelOptions */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
