On Tue, Oct 4, 2011 at 10:55 AM, Bruce Momjian <bruce@momjian.us> wrote:
>> It seems both ugly and unnecessary to declare dump_config_variable as
>> char[MAXPGPATH]. MAXPGPATH doesn't seem like the right length for a
>> buffer intended to hold a GUC name, but in fact I don't think you need
>> a buffer at all. I think you can just declare it as char * and say
>> dump_config_variable = optarg. getopt() doesn't overwrite the input
>> argument vector, does it?
>
> Well, as I remember, it writes a null byte at the end of the argument
> and then passes the pointer to the start --- when it advances to the
> next argument, it removes the null, so the pointer might still be valid,
> but does not have proper termination (or maybe that's what strtok()
> does). However, I can find no documentation that mentions this
> restriction, so perhaps it is old and no longer valid.
>
> If you look in our code you will see tons of cases of:
>
> user = strdup(optarg);
> pg_data = xstrdup(optarg);
> my_opts->dbname = mystrdup(optarg);
>
> However, I see other cases where we just assign optarg and optarg is a
> string, e.g. pg_dump:
>
> username = optarg;
>
> Doing a Google search shows both types of coding in random code pieces.
>
> Are the optarg string duplication calls unnecessary and can be removed?
> Either that, or we need to add some.
Well, if you want to keep it, I'd suggest using malloc() to get an
appropriate size buffer (not palloc) rather than using some arbitrary
constant for the length.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company