Alvaro Herrera wrote: > KaiGai Kohei wrote: > >> (1) Who/Where should allocate a string area? >> >> + /* Note that this assumes that the variable is already allocated! */ >> + #define HANDLE_STRING_RELOPTION(optname, var, option) \ >> + if (HAVE_RELOPTION(optname, option)) \ >> + { \ >> + strcpy(var, \ >> + option.isset ? option.values.string_val : \ >> + ((relopt_string *) option.gen)->default_val); \ >> + continue; \ >> + } >> >> I think HANDLE_STRING_RELOPTION() should allocate a string area via >> pstrdup(). If we have to put individual pstrdup() for each reloptions, >> it will make noisy listing of codes. >> >> How do you think: >> >> #define HANDLE_STRING_RELOPTION(optname, var, option) \ >> if (HAVE_RELOPTION(optname, option)) \ >> { \ >> char *tmp = (option.isset ? option.values.string_val : \ >> ((relopt_string *) option.gen)->default_val); \ >> var = pstrdup(tmp); \ >> continue; \ >> } > > Well, that's precisely the problem with string options. If we want > memory to be freed properly, we can only allocate a single chunk which > is what's going to be stored under the rd_options bytea pointer. > Allocating separately doesn't work because we need to rebuild the > relcache entry (freeing it and allocating a new one) when it is > invalidated for whatever reason. Since the relcache code cannot follow > a pointer stored in the bytea area, this would result in a permanent > memory leak. > > So the rule I came up with is that the caller is responsible for > allocating it -- but it must be inside the bytea area to be returned. > Below is a sample amoptions routine to show how it works. Note that > this is exactly why I said that only a single string option can be > supported.
If the caller allocates a surplus area to store string on tail of the StdRdOptions (or others), the string option can be represented as an offset value and should be accessed via macros, like: struct StdRdOptions { int32 vl_len_; int fillfactor; int test_option_a; /* indicate offset of the surplus area from head */ int test_option_b; /* of this structure. */ /* in actually surplus area is allocated here */ }; #define GetOptionString(ptr, ofs) (ofs==0 ? NULL : ((char *)(ptr) + (ptr)->(ofs))) We can access string options as follows: elog(NOTICE, "test_option_a is %s", GetOptionString(RdOpts, test_option_a)); elog(NOTICE, "test_option_b is %s", GetOptionString(RdOpts, test_option_b)); It enables to store multiple string options, and represent NULL string. If possible, HANDLE_STRING_RELOPTION() should be able to manage the head of unused surplus area and put the given val its offset automatically. I think using a macro to access string option is more proper restriction than mutually exclusive string options. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kai...@ak.jp.nec.com> -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers