Solaris getopt_long and PostgreSQL
About a year ago, you talked to the PostgreSQL people about some problem with Solaris getopt_long, and they changed the build to use the internal getopt_long instead of the Solaris one?
What was the problem with Solaris getopt_long? Does the problem still exist in Solaris 10?
My users are unhappy at the change, since normal getopt_long reorders the args, and apparently the built-in one doesn't, so "psql database -p port" no longer works, since it treats -p as the user name.
I don't know if I should revert that change, or port netBSD getopt_long and replace the PostgreSQL one with that.
Dne 17.03.09 16:38, Chuck McDevitt napsal(a):
About a year ago, you talked to the PostgreSQL people about some problem
with Solaris getopt_long, and they changed the build to use the internal
getopt_long instead of the Solaris one?
The problem was with getopt which is works little bit differently when -
is specified in optstring. If you look in POSIX it does not define how
getopt should work in this case. Solaris getopt implementation has
extension for long args. Unfortunately PostgreSQL clashes with it,
because it does not use getopt_long for long argument, but getopt(argc,
argv,"xy-").
What was the problem with Solaris getopt_long? Does the problem still
exist in Solaris 10?
yes and it will exist forever, because it is public API.
see man -s3C getopt
My users are unhappy at the change, since normal getopt_long reorders
the args, and apparently the built-in one doesn’t, so “psql database –p
port” no longer works, since it treats –p as the user name.
I understand, I'm not happy too :(, but how Tom mentioned it has been
never supposed to work. See documentation
I don’t know if I should revert that change, or port netBSD getopt_long
and replace the PostgreSQL one with that.
getopt_long is OK. Problem is getopt. getopt in core is currently taken
from *BSD but it could be updated.
One possible solution should be to use internal getopt only for postgres
binary and for other to use solaris libc version.
Zdenek
Dne 17.03.09 17:13, Zdenek Kotala napsal(a):
I don’t know if I should revert that change, or port netBSD
getopt_long and replace the PostgreSQL one with that.getopt_long is OK. Problem is getopt. getopt in core is currently taken
from *BSD but it could be updated.One possible solution should be to use internal getopt only for postgres
binary and for other to use solaris libc version.
I'm now looking into the code and most binaries uses getopt_long which
works fine. I think getopt_long can be used from Solaris libc. Only
postgres and pg_resetxlog uses getopt. I think nobody has problem with
postgres, because it is called from pg_ctl or other start/stop script
and pg_resetxlog should be modified to use getopt_long.
Comments?
Zdenek
Dne 17.03.09 17:24, Zdenek Kotala napsal(a):
Dne 17.03.09 17:13, Zdenek Kotala napsal(a):
I don’t know if I should revert that change, or port netBSD
getopt_long and replace the PostgreSQL one with that.getopt_long is OK. Problem is getopt. getopt in core is currently
taken from *BSD but it could be updated.One possible solution should be to use internal getopt only for
postgres binary and for other to use solaris libc version.I'm now looking into the code and most binaries uses getopt_long which
works fine. I think getopt_long can be used from Solaris libc. Only
postgres and pg_resetxlog uses getopt. I think nobody has problem with
postgres, because it is called from pg_ctl or other start/stop script
and pg_resetxlog should be modified to use getopt_long.
There is a patch, please test it if it satisfy your expectation. Do not
forget run autoconf.
Zdenek
Attachments:
getopt.patchtext/plain; name=getopt.patchDownload
*** pgsql.ed0f426df3ee/configure.in 2009-03-17 17:42:16.131828743 +0100
--- pgsql/configure.in 2009-03-17 17:38:58.639284000 +0100
***************
*** 1274,1280 ****
# our versions on that platform.
if test "$PORTNAME" = "solaris"; then
AC_LIBOBJ(getopt)
- AC_LIBOBJ(getopt_long)
elif test x"$ac_cv_type_struct_option" = xyes ; then
AC_REPLACE_FUNCS([getopt_long])
else
--- 1274,1279 ----
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
[ use Solaris' version of getopt_long ]
The reason not to do that was discussed in this thread:
http://archives.postgresql.org//pgsql-patches/2008-02/msg00075.php
While Chuck is certainly free to build his local copy however he wants,
I don't think we're going to revert this change in the community
sources.
regards, tom lane
What about the idea of updating our getopt and getopt_long to a more modern version by porting over netBSD getopt?
The current situation is confusing for users, as "psql databasename -p port" type of calls works on almost all platforms, but not on those using the internal getopt/getopt_long. For those, you get "-p" is not a valid user.
This is because MAC, BSD and GNU getopt_long permutes the arguments, and our getopt_long does not.
Show quoted text
-----Original Message-----
From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
Sent: Tuesday, March 17, 2009 11:02 AM
To: Zdenek Kotala
Cc: Chuck McDevitt; pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] Solaris getopt_long and PostgreSQLZdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
[ use Solaris' version of getopt_long ]
The reason not to do that was discussed in this thread:
http://archives.postgresql.org//pgsql-patches/2008-02/msg00075.php
While Chuck is certainly free to build his local copy however he wants,
I don't think we're going to revert this change in the community
sources.regards, tom lane
Chuck McDevitt <cmcdevitt@greenplum.com> writes:
This is because MAC, BSD and GNU getopt_long permutes the arguments, and our getopt_long does not.
AFAIK those all work by scribbling on the original argv[] array, a
behavior that seems pretty risky from a portability standpoint.
Since our port/ module is only going to get used on old platforms with
no or broken getopt_long(), it needs to be pretty conservative about
what it assumes the system environment can handle.
regards, tom lane
Perhaps I could use the same test pg_status uses to decide PS_USE_CHANGE_ARGV and PS_USE_CLOBBER_ARGV?
Any obviously, we don't just use ours for platforms with no or broken getopt_long, since we are talking Solaris (which has a bug in getopt, but getopt_long works fine)
Show quoted text
-----Original Message-----
From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
Sent: Tuesday, March 17, 2009 11:26 AM
To: Chuck McDevitt
Cc: Zdenek Kotala; pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] Solaris getopt_long and PostgreSQLChuck McDevitt <cmcdevitt@greenplum.com> writes:
This is because MAC, BSD and GNU getopt_long permutes the arguments,
and our getopt_long does not.
AFAIK those all work by scribbling on the original argv[] array, a
behavior that seems pretty risky from a portability standpoint.
Since our port/ module is only going to get used on old platforms with
no or broken getopt_long(), it needs to be pretty conservative about
what it assumes the system environment can handle.regards, tom lane
Dne 17.03.09 19:48, Chuck McDevitt napsal(a):
Any obviously, we don't just use ours for platforms with no or broken getopt_long,
since we are talking Solaris (which has a bug in getopt, but
getopt_long works fine)
Just for clarification:
It is not bug in Solaris. It is Solaris' getopt extension for long arg
processing from days when getopt_long did not exist (hmm it seems that
it is still not in POSIX :( ). By my opinion PostgreSQL does not work
correctly here, because it uses construction which is marked as a
implementation depended in POSIX standard.
See:
http://www.opengroup.org/onlinepubs/009695399/functions/getopt.html
http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap12.html#tag_12_02
Zdenek
On Tuesday 17 March 2009 20:02:14 Tom Lane wrote:
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
[ use Solaris' version of getopt_long ]
The reason not to do that was discussed in this thread:
http://archives.postgresql.org//pgsql-patches/2008-02/msg00075.php
That discussion was about option parsing in postgres/postmaster, which does
not use getopt_long at all.
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
Dne 17.03.09 19:48, Chuck McDevitt napsal(a):
Any obviously, we don't just use ours for platforms with no or broken getopt_long,
since we are talking Solaris (which has a bug in getopt, but
getopt_long works fine)
Just for clarification:
It is not bug in Solaris.
Well, "bug" in the sense that it doesn't do what we want it to.
After reviewing this thread and the one that led up to the 8.3 behavior,
it seems clear that we failed to draw a distinction between getopt and
getopt_long when we should have. We don't like Solaris' getopt but
there seems no reason not to use Solaris' getopt_long. So Zdenek's
suggestion to change configure seems the correct fix, and I've done
that.
regards, tom lane
I wrote:
After reviewing this thread and the one that led up to the 8.3 behavior,
it seems clear that we failed to draw a distinction between getopt and
getopt_long when we should have. We don't like Solaris' getopt but
there seems no reason not to use Solaris' getopt_long. So Zdenek's
suggestion to change configure seems the correct fix, and I've done
that.
So my reward for that is to find that every one of the Solaris 11
buildfarm members is failing today:
pg_regress: initdb failed
Examine /export/home/tmp/pg-test/build-suncc/HEAD/pgsql.6320/src/test/regress/log/initdb.log for the reason.
Command was: "/export/home/tmp/pg-test/build-suncc/HEAD/pgsql.6320/src/test/regress/./tmp_check/install//export/home/tmp/pg-test/build-suncc/HEAD/inst/bin/initdb" -D "/export/home/tmp/pg-test/build-suncc/HEAD/pgsql.6320/src/test/regress/./tmp_check/data" -L "/export/home/tmp/pg-test/build-suncc/HEAD/pgsql.6320/src/test/regress/./tmp_check/install//export/home/tmp/pg-test/build-suncc/HEAD/inst/share/postgresql" --noclean --no-locale > "/export/home/tmp/pg-test/build-suncc/HEAD/pgsql.6320/src/test/regress/log/initdb.log" 2>&1
make: *** [check] Error 2
================== pgsql.6320/src/test/regress/log/initdb.log ===================
initdb: too many command-line arguments (first is "-L")
Try "initdb --help" for more information.
Running in noclean mode. Mistakes will not be cleaned up.
Apparently the system version of getopt_long is broken on Solaris 11.
My patience for this grows short.
regards, tom lane
Tom Lane píše v so 28. 03. 2009 v 14:36 -0400:
I wrote:
After reviewing this thread and the one that led up to the 8.3 behavior,
it seems clear that we failed to draw a distinction between getopt and
getopt_long when we should have. We don't like Solaris' getopt but
there seems no reason not to use Solaris' getopt_long. So Zdenek's
suggestion to change configure seems the correct fix, and I've done
that.So my reward for that is to find that every one of the Solaris 11
buildfarm members is failing today:
I'm going to look on that.
Zdenek
Tom Lane píše v so 28. 03. 2009 v 14:36 -0400:
Apparently the system version of getopt_long is broken on Solaris 11.
My patience for this grows short.
It is not problem with getopt_long itself, but with symbol overriding.
getopt_long uses optind and so on from libc, but e.g. initdb takes
optind which is defined in port/getopt.c. It causes problem in following
code:
02607 /* Non-option argument specifies data directory */
02608 if (optind < argc)
which compares different variable.
I think it is general problem. HAVE_GETOPT_LONG cannot be defined when
HAVE_GETOPT_H is not defined.
I'm sorry for my previous patch. I had to make some mistake in
autoconf/make magic.
I attached a fix. Only problem what I see there is getopt_long.h which
contains
#ifdef HAVE_GETOPT_H
#include <getopt.h>
#endif
but getopt.h is required for getopt_long(). Fortunately, content is
similar with getopt_long.h and there is no problem with it on Solaris.
Zdenek
Attachments:
getopt.patchtext/x-patch; charset=UTF-8; name=getopt.patchDownload
*** pgsql.orig.2ecfaec29a72/src/port/getopt.c 2009-03-30 22:06:31.183510859 +0200
--- pgsql.orig/src/port/getopt.c 2009-03-30 21:34:04.697048053 +0200
***************
*** 36,47 ****
static char sccsid[] = "@(#)getopt.c 8.3 (Berkeley) 4/27/95";
#endif /* LIBC_SCCS and not lint */
!
! int opterr = 1, /* if error message should be printed */
! optind = 1, /* index into parent argv vector */
! optopt, /* character checked for validity */
! optreset; /* reset getopt */
char *optarg; /* argument associated with option */
#define BADCH (int)'?'
#define BADARG (int)':'
--- 36,52 ----
static char sccsid[] = "@(#)getopt.c 8.3 (Berkeley) 4/27/95";
#endif /* LIBC_SCCS and not lint */
! /* In situation when we use getopt_long from libc, we needs to use libc variable,
! * else it causes symbol overriding and optind contains nonsens.
! */
! #ifndef HAVE_GETOPT_LONG
! int opterr = 1; /* if error message should be printed */
! int optind = 1; /* index into parent argv vector */
! int optopt; /* character checked for validity */
char *optarg; /* argument associated with option */
+ #endif
+ static int optreset; /* reset getopt */
+
#define BADCH (int)'?'
#define BADARG (int)':'
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
I attached a fix. Only problem what I see there is getopt_long.h which
contains
I'm more concerned about the "static int optreset", which is 100%
guaranteed to break things.
regards, tom lane
Dne 31.03.09 04:55, Tom Lane napsal(a):
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
I attached a fix. Only problem what I see there is getopt_long.h which
containsI'm more concerned about the "static int optreset", which is 100%
guaranteed to break things.
Yeah correct, I overlooked that optreset is also defined as a extern.
There is updated patch.
Thanks Zdenek
Attachments:
getopt_02.patchtext/plain; name=getopt_02.patchDownload
*** pgsql.c598eae30145/src/port/getopt.c 2009-03-31 13:31:56.896353577 +0200
--- pgsql/src/port/getopt.c 2009-03-31 12:22:04.619485958 +0200
***************
*** 36,47 ****
static char sccsid[] = "@(#)getopt.c 8.3 (Berkeley) 4/27/95";
#endif /* LIBC_SCCS and not lint */
!
! int opterr = 1, /* if error message should be printed */
! optind = 1, /* index into parent argv vector */
! optopt, /* character checked for validity */
! optreset; /* reset getopt */
char *optarg; /* argument associated with option */
#define BADCH (int)'?'
#define BADARG (int)':'
--- 36,54 ----
static char sccsid[] = "@(#)getopt.c 8.3 (Berkeley) 4/27/95";
#endif /* LIBC_SCCS and not lint */
! /* In situation when we use getopt_long from libc, we needs to use libc variable,
! * else it causes symbol overriding and optind contains nonsens.
! */
! #ifndef HAVE_GETOPT_LONG
! int opterr = 1; /* if error message should be printed */
! int optind = 1; /* index into parent argv vector */
! int optopt; /* character checked for validity */
char *optarg; /* argument associated with option */
+ #endif
+
+ #ifndef HAVE_INT_OPTRESET
+ int optreset; /* reset getopt */
+ #endif
#define BADCH (int)'?'
#define BADARG (int)':'
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
Yeah correct, I overlooked that optreset is also defined as a extern.
There is updated patch.
On looking at this I still can't see how it's not broken. You are
effectively assuming that getopt_long.c must define those variables.
But surely getopt_long.c should be assuming that getopt.c defines them.
Aren't we likely to end up with the situation that everyone is
extern'ing them?
What appears to me to be happening is that Solaris' linker is failing to
merge global variable definitions when it should (must) do so. We need
to find out why rather than solve it with a patch that will certainly
break other platforms.
If you can't come up with a real solution, we might have to do this
with "#ifndef SOLARIS" or something similar.
regards, tom lane
Dne 31.03.09 18:21, Tom Lane napsal(a):
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
Yeah correct, I overlooked that optreset is also defined as a extern.
There is updated patch.On looking at this I still can't see how it's not broken. You are
effectively assuming that getopt_long.c must define those variables.
No I assuming that when we use getopt_long from libc then libc already
has these variables defined. It must.
But surely getopt_long.c should be assuming that getopt.c defines them.
Aren't we likely to end up with the situation that everyone is
extern'ing them?
Yeah, getopt_long assumes that optind, opterr and so on are already
defined by getopt. If you look in current implementation in BDS you can
see that there is getopt_internal() and getopt(), getopt_long are only
wrapper.
The main problem what I see here is that getopt and getopt_long works
together. Use one from system and one ported is not good idea. I think
best solution is to port new BSD version into postgreSQL and use both
function from libc version or ported versin. Mixing then is risky.
See also e.g. initdb.c, there is declaration of opterr...
What appears to me to be happening is that Solaris' linker is failing to
merge global variable definitions when it should (must) do so. We need
to find out why rather than solve it with a patch that will certainly
break other platforms.
Linker cannot do it in general. Libc contains these variables and
compiled code points to them. When you declare them again into your
application you override them but only for your application, not for libc.
If you can't come up with a real solution, we might have to do this
with "#ifndef SOLARIS" or something similar.
I prefer another solution then this, but it is also possible.
Personally prefer to port new getopt_long. It is benefit also for win users.
Zdenek
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
The main problem what I see here is that getopt and getopt_long works
together. Use one from system and one ported is not good idea.
Well, the expected (and pretty-well-tested) case is that your system has
getopt but not getopt_long. I don't see any reason why using ported
getopt_long in that case is "not good idea".
I agree that substituting getopt without substituting getopt_long is a
tad risky, and probably hasn't been tested anyplace else previously.
It may well be that we should revert to the previous state of affairs
where we don't trust Solaris for either function.
I think best solution is to port new BSD version into postgreSQL and
use both function from libc version or ported versin.
I'm not sure which part of "no" you didn't understand. Changing the
contents of argv[] is going to be system-specific and there is no reason
to believe that a BSD implementation will work everywhere.
regards, tom lane
Tom Lane píše v út 31. 03. 2009 v 13:10 -0400:
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
The main problem what I see here is that getopt and getopt_long works
together. Use one from system and one ported is not good idea.Well, the expected (and pretty-well-tested) case is that your system has
getopt but not getopt_long. I don't see any reason why using ported
getopt_long in that case is "not good idea".
I'm looking on to POSIX and all opt* variable are specified there and
getopt_long use only what is specified. It should be OK.
I agree that substituting getopt without substituting getopt_long is a
tad risky, and probably hasn't been tested anyplace else previously.
It seems to me, that optind,... is same case lake optreset. I'm thinking
to add HAVE_INT_OPTIND macro (similar to HAVE_INT_OPTRESET) and use it
instead of HAVE_GETOPT_LONG in my previous patch.
Another possibility is to rewrite postgres(and pg_resetxlog) to use
getopt_long() instead of getopt().
It may well be that we should revert to the previous state of affairs
where we don't trust Solaris for either function.
I would like to solve it rather then revert back.
Zdenek
Zdenek Kotala píše v út 31. 03. 2009 v 21:25 +0200:
Another possibility is to rewrite postgres(and pg_resetxlog) to use
getopt_long() instead of getopt().
Attached patch rewrites postgres to use getopt_long instead of getopt.
Patch also removes configure part for Solaris related to getopt.
Zdenek
Attachments:
getopt.patchtext/x-patch; charset=UTF-8; name=getopt.patchDownload
diff -Nrc pgsql_getopt.2ecfaec29a72/configure.in pgsql_getopt/configure.in
*** pgsql_getopt.2ecfaec29a72/configure.in 2009-04-05 23:12:35.718886756 +0200
--- pgsql_getopt/configure.in 2009-04-05 23:12:35.761770812 +0200
***************
*** 1276,1287 ****
AC_LIBOBJ(getopt_long)
fi
- # Solaris' getopt() doesn't do what we want for long options, so always use
- # our version on that platform.
- if test "$PORTNAME" = "solaris"; then
- AC_LIBOBJ(getopt)
- fi
-
# Win32 support
if test "$PORTNAME" = "win32"; then
AC_REPLACE_FUNCS(gettimeofday)
--- 1276,1281 ----
diff -Nrc pgsql_getopt.2ecfaec29a72/src/backend/bootstrap/bootstrap.c pgsql_getopt/src/backend/bootstrap/bootstrap.c
*** pgsql_getopt.2ecfaec29a72/src/backend/bootstrap/bootstrap.c 2009-04-05 23:12:35.723955129 +0200
--- pgsql_getopt/src/backend/bootstrap/bootstrap.c 2009-04-05 23:12:35.762270558 +0200
***************
*** 17,25 ****
#include <time.h>
#include <unistd.h>
#include <signal.h>
! #ifdef HAVE_GETOPT_H
! #include <getopt.h>
! #endif
#include "access/genam.h"
#include "access/heapam.h"
--- 17,23 ----
#include <time.h>
#include <unistd.h>
#include <signal.h>
! #include "getopt_long.h"
#include "access/genam.h"
#include "access/heapam.h"
***************
*** 38,43 ****
--- 36,42 ----
#include "tcop/tcopprot.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
+ #include "utils/guc.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
#include "utils/tqual.h"
***************
*** 208,213 ****
--- 207,214 ----
int flag;
AuxProcType auxType = CheckerProcess;
char *userDoption = NULL;
+ struct option *optlist;
+ int optindex;
/*
* initialize globals
***************
*** 247,253 ****
argc--;
}
! while ((flag = getopt(argc, argv, "B:c:d:D:Fr:x:-:")) != -1)
{
switch (flag)
{
--- 248,256 ----
argc--;
}
! optlist = GetLongOptionList();
!
! while ((flag = getopt_long(argc, argv, "B:c:d:D:Fr:x:", optlist, &optindex)) != -1)
{
switch (flag)
{
***************
*** 280,286 ****
auxType = atoi(optarg);
break;
case 'c':
- case '-':
{
char *name,
*value;
--- 283,288 ----
***************
*** 288,293 ****
--- 290,296 ----
ParseLongOption(optarg, &name, &value);
if (!value)
{
+ free(optlist);
if (flag == '-')
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
***************
*** 306,311 ****
--- 309,317 ----
free(value);
break;
}
+ case 1 :
+ SetConfigOption(optlist[optindex].name, optarg, PGC_POSTMASTER, PGC_S_ARGV);
+ break;
default:
write_stderr("Try \"%s --help\" for more information.\n",
progname);
***************
*** 313,318 ****
--- 319,325 ----
break;
}
}
+ free(optlist);
if (argc != optind)
{
diff -Nrc pgsql_getopt.2ecfaec29a72/src/backend/postmaster/postmaster.c pgsql_getopt/src/backend/postmaster/postmaster.c
*** pgsql_getopt.2ecfaec29a72/src/backend/postmaster/postmaster.c 2009-04-05 23:12:35.729964754 +0200
--- pgsql_getopt/src/backend/postmaster/postmaster.c 2009-04-05 23:12:35.762704640 +0200
***************
*** 83,92 ****
#ifdef HAVE_SYS_SELECT_H
#include <sys/select.h>
#endif
!
! #ifdef HAVE_GETOPT_H
! #include <getopt.h>
! #endif
#ifdef USE_BONJOUR
#include <DNSServiceDiscovery/DNSServiceDiscovery.h>
--- 83,89 ----
#ifdef HAVE_SYS_SELECT_H
#include <sys/select.h>
#endif
! #include "getopt_long.h"
#ifdef USE_BONJOUR
#include <DNSServiceDiscovery/DNSServiceDiscovery.h>
***************
*** 116,121 ****
--- 113,119 ----
#include "tcop/tcopprot.h"
#include "utils/builtins.h"
#include "utils/datetime.h"
+ #include "utils/guc.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
***************
*** 463,468 ****
--- 461,468 ----
int status;
char *userDoption = NULL;
int i;
+ struct option *optlist;
+ int optindex;
MyProcPid = PostmasterPid = getpid();
***************
*** 506,517 ****
opterr = 1;
/*
* Parse command-line options. CAUTION: keep this in sync with
* tcop/postgres.c (the option sets should not conflict) and with the
* common help() function in main/main.c.
*/
! while ((opt = getopt(argc, argv, "A:B:c:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1)
{
switch (opt)
{
--- 506,518 ----
opterr = 1;
+ optlist = GetLongOptionList();
/*
* Parse command-line options. CAUTION: keep this in sync with
* tcop/postgres.c (the option sets should not conflict) and with the
* common help() function in main/main.c.
*/
! while ((opt = getopt_long(argc, argv, "A:B:c:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:", optlist, &optindex )) != -1)
{
switch (opt)
{
***************
*** 644,650 ****
break;
case 'c':
- case '-':
{
char *name,
*value;
--- 645,650 ----
***************
*** 671,676 ****
--- 671,680 ----
break;
}
+ case 1:
+ SetConfigOption(optlist[optindex].name, optarg, PGC_POSTMASTER, PGC_S_ARGV);
+ break;
+
default:
write_stderr("Try \"%s --help\" for more information.\n",
progname);
***************
*** 678,683 ****
--- 682,689 ----
}
}
+ free(optlist);
+
/*
* Postmaster accepts no non-option switch arguments.
*/
diff -Nrc pgsql_getopt.2ecfaec29a72/src/backend/tcop/postgres.c pgsql_getopt/src/backend/tcop/postgres.c
*** pgsql_getopt.2ecfaec29a72/src/backend/tcop/postgres.c 2009-04-05 23:12:35.739820636 +0200
--- pgsql_getopt/src/backend/tcop/postgres.c 2009-04-05 23:12:35.763135706 +0200
***************
*** 31,39 ****
#include <sys/time.h>
#include <sys/resource.h>
#endif
! #ifdef HAVE_GETOPT_H
! #include <getopt.h>
! #endif
#ifndef HAVE_GETRUSAGE
#include "rusagestub.h"
--- 31,37 ----
#include <sys/time.h>
#include <sys/resource.h>
#endif
! #include "getopt_long.h"
#ifndef HAVE_GETRUSAGE
#include "rusagestub.h"
***************
*** 69,74 ****
--- 67,73 ----
#include "utils/memutils.h"
#include "utils/ps_status.h"
#include "utils/snapmgr.h"
+ #include "utils/guc.h"
#include "mb/pg_wchar.h"
***************
*** 2859,2864 ****
--- 2858,2865 ----
StringInfoData input_message;
sigjmp_buf local_sigjmp_buf;
volatile bool send_ready_for_query = true;
+ struct option *optlist;
+ int optindex;
#define PendingConfigOption(name,val) \
(guc_names = lappend(guc_names, pstrdup(name)), \
***************
*** 2940,2946 ****
* postmaster/postmaster.c (the option sets should not conflict) and with
* the common help() function in main/main.c.
*/
! while ((flag = getopt(argc, argv, "A:B:c:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:v:W:y:-:")) != -1)
{
switch (flag)
{
--- 2941,2949 ----
* postmaster/postmaster.c (the option sets should not conflict) and with
* the common help() function in main/main.c.
*/
! optlist = GetLongOptionList();
!
! while ((flag = getopt_long(argc, argv, "A:B:c:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:v:W:y:", optlist, &optindex)) != -1)
{
switch (flag)
{
***************
*** 3074,3080 ****
SetConfigOption("post_auth_delay", optarg, ctx, gucsource);
break;
-
case 'y':
/*
--- 3077,3082 ----
***************
*** 3092,3098 ****
break;
case 'c':
- case '-':
{
char *name,
*value;
--- 3094,3099 ----
***************
*** 3100,3105 ****
--- 3101,3107 ----
ParseLongOption(optarg, &name, &value);
if (!value)
{
+ free(optlist);
if (flag == '-')
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
***************
*** 3126,3136 ****
--- 3128,3150 ----
break;
}
+ case 1:
+ /*
+ * If a SUSET option, must postpone evaluation, unless we
+ * are still reading secure switches.
+ */
+ if (ctx == PGC_BACKEND && IsSuperuserConfigOption(optlist[optindex].name))
+ PendingConfigOption(optlist[optindex].name, optarg);
+ else
+ SetConfigOption(optlist[optindex].name, optarg, ctx, gucsource);
+ break;
+
default:
errs++;
break;
}
}
+ free(optlist);
/*
* Process any additional GUC variable settings passed in startup packet.
diff -Nrc pgsql_getopt.2ecfaec29a72/src/backend/utils/misc/guc.c pgsql_getopt/src/backend/utils/misc/guc.c
*** pgsql_getopt.2ecfaec29a72/src/backend/utils/misc/guc.c 2009-04-05 23:12:35.758577351 +0200
--- pgsql_getopt/src/backend/utils/misc/guc.c 2009-04-05 23:12:35.763797096 +0200
***************
*** 26,31 ****
--- 26,33 ----
#include <syslog.h>
#endif
+ #include "getopt_long.h"
+
#include "access/gin.h"
#include "access/transam.h"
#include "access/twophase.h"
***************
*** 6872,6877 ****
--- 6874,6904 ----
#endif /* EXEC_BACKEND */
+ struct option *
+ GetLongOptionList()
+ {
+ struct option *opt;
+ struct config_generic *conf;
+ int i;
+ int numOpts;
+
+ numOpts = GetNumConfigOptions();
+
+ opt = malloc( sizeof(struct option) * (numOpts + 1) );
+
+ for (i = 0; i < numOpts; i++)
+ {
+ conf = guc_variables[i];
+ opt[i].name = conf->name;
+ opt[i].has_arg = TRUE;
+ opt[i].flag = NULL;
+ opt[i].val = 1;
+ }
+
+ memset( &opt[numOpts], 0, sizeof(struct option) );
+ return opt;
+ }
+
/*
* A little "long argument" simulation, although not quite GNU
* compliant. Takes a string of the form "some-option=some value" and
diff -Nrc pgsql_getopt.2ecfaec29a72/src/include/utils/guc.h pgsql_getopt/src/include/utils/guc.h
*** pgsql_getopt.2ecfaec29a72/src/include/utils/guc.h 2009-04-05 23:12:35.759983257 +0200
--- pgsql_getopt/src/include/utils/guc.h 2009-04-05 23:12:35.764155507 +0200
***************
*** 256,261 ****
--- 256,262 ----
extern int NewGUCNestLevel(void);
extern void AtEOXact_GUC(bool isCommit, int nestLevel);
extern void BeginReportingGUCOptions(void);
+ extern struct option *GetLongOptionList();
extern void ParseLongOption(const char *string, char **name, char **value);
extern bool parse_int(const char *value, int *result, int flags,
const char **hintmsg);
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
Zdenek Kotala píše v út 31. 03. 2009 v 21:25 +0200:
Another possibility is to rewrite postgres(and pg_resetxlog) to use
getopt_long() instead of getopt().
Attached patch rewrites postgres to use getopt_long instead of getopt.
Actually, I fooled around with it last night and seem to have fixed it
(buildfarm is all green today) by the expedient of not defining our own
optind etc. variables if the system supplies them. So that seemed like
a clean fix to me --- the old handling of optreset in particular was a
huge kluge, whereas it's clear how this code is supposed to work.
I don't think we need to mess around with changing our option parsing
logic, especially not to the extent that you propose here.
regards, tom lane
Tom Lane píše v ne 05. 04. 2009 v 17:44 -0400:
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
Zdenek Kotala píše v út 31. 03. 2009 v 21:25 +0200:
Another possibility is to rewrite postgres(and pg_resetxlog) to use
getopt_long() instead of getopt().Attached patch rewrites postgres to use getopt_long instead of getopt.
Actually, I fooled around with it last night and seem to have fixed it
(buildfarm is all green today) by the expedient of not defining our own
optind etc. variables if the system supplies them. So that seemed like
a clean fix to me --- the old handling of optreset in particular was a
huge kluge, whereas it's clear how this code is supposed to work.
Yeah, everything is green today. Thanks. Is it possible to backport it
at least to 8.3?
I don't think we need to mess around with changing our option parsing
logic, especially not to the extent that you propose here.
When previous solution works well on all platforms there is no reason to
use getopt_long.
Thanks Zdenek