* From: MauMau [mailto:maumau...@gmail.com]

> From: "Christian Ullrich" <ch...@chrullrich.net>

> > OK, here is the first draft against current master. It builds on Windows
> > with VS 2012 and on FreeBSD 10 with clang 3.3. I ran the regression
> > tests on Windows, they all pass.
> >
> > The changed behavior is limited to Windows, where it now silently
> > ignores Ctrl-C and Ctrl-Break when started via pg_ctl start.
> >
> > I don't think there is currently any support for switch-type long
> > options, so rather than invent my own, I squeezed the two lines I added
> > into postmaster.c where they fit best; unfortunately, the result is
> > quite ugly. I'll be happy to refine that if someone can give me a hint
> > on how to do it.
> 
> Overall, the patch seems good as it is based on the discussion.  I found a
> few problems:

Thank you for the review. I will rebase, retest, and resubmit as soon as I find 
the time, certainly sometime this week.

> (2)
> Although I haven't tried, doesn't pg_ctl start fail on non-Windows
> platforms
> because of the following check?
> 
> !        if (opt == '-')
> !         ereport(ERROR,
> !           (errcode(ERRCODE_SYNTAX_ERROR),
> !            errmsg("--%s requires a value",
> !             optarg)));

On non-Windows platforms, the --background option is not passed, and the option 
handling is unmodified except for an additional pair of braces. The postmaster 
does not pass the option to its children on any platform.

> And, in the postgres reference page,
> 
> http://www.postgresql.org/docs/devel/static/app-postgres.html
> 
> there's a paragraph:
> 
> "The -- options will not work on FreeBSD or OpenBSD. Use -c instead. This
> is a bug in the affected operating systems; a future release of PostgreSQL
> willprovide a workaround if this is not fixed."
> 
> Would --background work on FreeBSD and OpenBSD (i.e. would pg_ctl start
> succeed)?  I don't have access to those platforms.

pg_ctl does not pass the option anywhere but on Windows, and postmaster.c does 
not recognize it anywhere else. If it is encountered on a platform where it 
does not make sense, it will be treated like any other (unknown) long option.

This is actually the weakest point of the existing patch, in my opinion. 
Jamming the long option handling into postmaster.c by way of #ifdef WIN32 feels 
wrong, but I could not figure out a better way to do it.

> (3)
> --background will also be used by restart subcommand, won't it?
> 
> +     in a console window. It is used automatically by
> +     <command>pg_ctl</command> when called with the
> +     <option>start</option> subcommand.

Restart is implemented as stop/start, so, yes.

-- 
Christian

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to