Robert Haas wrote: > On Tue, Oct 4, 2011 at 10:55 AM, Bruce Momjian <br...@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.
The new code does strdup(), which will match what is passed. -- Bruce Momjian <br...@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
