Re: Bug with pg_ctl -w/wait and config-only directories

Robert Haas wrote:
> 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.

The new code does strdup(), which will match what is passed.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +