Change the way string relopts are allocated.
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Tue, 9 Aug 2011 12:25:44 +0000 (15:25 +0300)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Tue, 9 Aug 2011 12:25:44 +0000 (15:25 +0300)
Don't try to allocate the default value for a string relopt in the same
palloc chunk as the relopt_string struct. That didn't work too well if you
added a built-in string relopt in the stringRelOpts array, as it's not
possible to have an initializer for a variable length struct in C. This
makes the code slightly simpler too.

While we're at it, move the call to validator function in
add_string_reloption to before the allocation, so that if someone does pass
a bogus default value, we don't leak memory.

src/backend/access/common/reloptions.c
src/include/access/reloptions.h

index 465742556f5b990b9bff095e757802477c01d299..900b222865e15ae482d0ac46b8ed8a159275de23 100644 (file)
@@ -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);
 }
 
index c7709cc058956f0cf416a310e8f81770f25e0d28..14f50345bbf4faf3f354bce3864edee33726a27e 100644 (file)
@@ -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 */