Re: windows / initdb oddness

Andrew Dunstan wrote:

>
>
> Magnus Hagander wrote:
>
>>>>
>>>> The solution would be put --restrictedexec earlier on the
>>>
>>> new command
>>>
>>>> line. I'll work on that.
>>>>
>>>
>>> The probem is apparently the one I identified above, and is fixed by
>>> the attached patch, which I will apply soon unless there are
>>> objections.
>>>
>>> As for why we saw this on loris but not snake, I suspect they might
>>> have different getopt libraries installed.
>>>
>>
>>
>> Isn't that just fixing the symptom and not the actual bug? In this case,
>> if we cause the bug, we should do this as well, but doesn't it crash the
>> same way if you *manually* put arguments in the "wrong order" on the
>> commandline? Like "inidb foo --no-locale" or somehting like that?
>>
>> (I still can't reproduce it on my machines, so I guess I have a better
>> getopt as well.)
>>
>>
>>
>>
>
> We don't promise that you can put the pgdata argument anywhere except
> at the end of the command line. In fact, our manual page requires it
> at the end. Even on systems with GNU getopt, if POSIXLY_CORRECT is set
> then processing would stop at the first non-getopt argument.
>
> So I can live with bombing, even if it's a bit unpleasant, in the case
> of  "initdb foo --no-locale", but we cannot *cause* that by appending
> a secret argument ourselves, so that "initdb foo" also bombs.
>
> The logic to detect and correct this in the general case before getopt
> is called is not worth the pain.
>


Thinking about this a tiny bit more, it struck me that by far the best
way to do this is to stop using a magic argument and use the environment
instead. Then we don't need to mangle the command line at all. This
actually results in less code, and should be more robust (mangling the
command line in Windows is dangerous and difficult because of quotes).

Trial patch below, although I don't have a Windows box handy to test it on.


cheers

andrew




? .deps
? initdb
Index: initdb.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/initdb/initdb.c,v
retrieving revision 1.110
diff -c -r1.110 initdb.c
*** initdb.c    18 Feb 2006 16:15:23 -0000    1.110
--- initdb.c    22 Feb 2006 16:10:28 -0000
***************
*** 95,103 ****
  static bool debug = false;
  static bool noclean = false;
  static bool show_setting = false;
- #ifdef WIN32
- static bool restricted_exec = false;
- #endif


  /* internal vars */
--- 95,100 ----
***************
*** 2427,2437 ****
          {"lc-messages", required_argument, NULL, 7},
          {"no-locale", no_argument, NULL, 8},
          {"auth", required_argument, NULL, 'A'},
!         {"pwprompt", no_argument, NULL, 'W'},
          {"pwfile", required_argument, NULL, 9},
- #ifdef WIN32
-         {"restrictedexec", no_argument, NULL, 10},
- #endif
          {"username", required_argument, NULL, 'U'},
          {"help", no_argument, NULL, '?'},
          {"version", no_argument, NULL, 'V'},
--- 2424,2431 ----
          {"lc-messages", required_argument, NULL, 7},
          {"no-locale", no_argument, NULL, 8},
          {"auth", required_argument, NULL, 'A'},
!         {"pwprompt", no_argument, NULL, 'W'},
          {"pwfile", required_argument, NULL, 9},
          {"username", required_argument, NULL, 'U'},
          {"help", no_argument, NULL, '?'},
          {"version", no_argument, NULL, 'V'},
***************
*** 2450,2455 ****
--- 2444,2452 ----
                                   * environment */
      char        bin_dir[MAXPGPATH];
      char       *pg_data_native;
+ #ifdef WIN32
+     char       *restrict_env;
+ #endif
      static const char *subdirs[] = {
          "global",
          "pg_xlog",
***************
*** 2540,2550 ****
              case 9:
                  pwfilename = xstrdup(optarg);
                  break;
- #ifdef WIN32
-             case 10:
-                 restricted_exec = true;
-                 break;
- #endif
              case 's':
                  show_setting = true;
                  break;
--- 2537,2542 ----
***************
*** 2556,2561 ****
--- 2548,2554 ----
          }
      }

+
      /* Non-option argument specifies data directory */
      if (optind < argc)
      {
***************
*** 2644,2659 ****
       * Before we execute another program, make sure that we are running with a
       * restricted token. If not, re-execute ourselves with one.
       */
!     if (!restricted_exec)
      {
          PROCESS_INFORMATION pi;
          char *cmdline;

          ZeroMemory(&pi, sizeof(pi));

!         cmdline = pg_malloc(strlen(GetCommandLine()) + 19);
!         strcpy(cmdline, GetCommandLine());
!         strcat(cmdline, " --restrictedexec");

          if (!CreateRestrictedProcess(cmdline, &pi))
          {
--- 2637,2654 ----
       * Before we execute another program, make sure that we are running with a
       * restricted token. If not, re-execute ourselves with one.
       */
!
!     if ((restrict_env = getenv("PG_RESTRICT_EXEC")) == NULL
!         || strcmp(restrict_env,"1") != 0)
      {
          PROCESS_INFORMATION pi;
          char *cmdline;

          ZeroMemory(&pi, sizeof(pi));

!         cmdline = x_strdup(GetCommandLine());
!
!         putenv("PG_RESTRICT_EXEC=1");

          if (!CreateRestrictedProcess(cmdline, &pi))
          {