Re: [PATCHES] [HACKERS] windows / initdb oddness

[Andrew Dunstan]

Andrew Dunstan wrote:


I wrote:



I will add some trace writes when I get a chance. I was rather hoping 
something would jump out at you, but obviously it hasn't, so I'll 
have to dig into it the slow way. *sigh*




Just eyeballing the code it looks to me like the problem is this line:

   strcat(cmdline, * --restrictedexec*);


which is appending an option type argument after the non-option argument.


That would exactly account for the failure when we call initdb foo 
but not initdb -D foo.


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.


cheers

andrew
Index: initdb.c
===
RCS file: /projects/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 -	1.110
--- initdb.c	22 Feb 2006 02:21:59 -
***
*** 2450,2455 
--- 2450,2460 
   * environment */
  	char		bin_dir[MAXPGPATH];
  	char	   *pg_data_native;
+ 
+ #ifdef WIN32
+ 	char  *orig_pgdata = NULL;
+ #endif
+ 
  	static const char *subdirs[] = {
  		global,
  		pg_xlog,
***
*** 2560,2565 
--- 2565,2573 
  	if (optind  argc)
  	{
  		pg_data = xstrdup(argv[optind]);
+ #ifdef WIN32
+ 		orig_pgdata = xstrdup(pg_data);
+ #endif
  		optind++;
  	}
  
***
*** 2648,2660 
  {
  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))
  {
  fprintf(stderr,Failed to re-exec with restricted token: %lu.\n, GetLastError());
--- 2656,2687 
  {
  PROCESS_INFORMATION pi;
  char *cmdline;
! 
  ZeroMemory(pi, sizeof(pi));
  
! cmdline = pg_malloc(strlen(GetCommandLine()) + 20);
  strcpy(cmdline, GetCommandLine());
! 
! 		if (orig_pgdata  != NULL)
! 		{
! 			/* find the LAST occurrence of the data arg on the command line */
! 			char *data_arg;
! 			char *next_pos;
! 			size_t orig_len = strlen(orig_pgdata);
! 
! 			data_arg=strstr(cmdline,orig_pgdata);
! 			while ((next_pos=strstr(data_arg+1,orig_pgdata)) != NULL)
! data_arg = next_pos;
! 			/* wipe it out so we can add the extra arg */
! 			*data_arg = '\0';
! 		}
! 		
! strcat(cmdline,  --restrictedexec );
  
+ 		/* put back original arg if we wiped it out above */
+ 		if (orig_pgdata  != NULL)
+ 			strcat(cmdline,orig_pgdata);
+ 
  if (!CreateRestrictedProcess(cmdline, pi))
  {
  fprintf(stderr,Failed to re-exec with restricted token: %lu.\n, GetLastError());

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

Re: [PATCHES] [HACKERS] windows / initdb oddness

[Magnus Hagander]

  I will add some trace writes when I get a chance. I was 
 rather hoping 
  something would jump out at you, but obviously it hasn't, so I'll 
  have to dig into it the slow way. *sigh*
 
 
 
  Just eyeballing the code it looks to me like the problem is 
 this line:
 
 strcat(cmdline, * --restrictedexec*);
 
 
  which is appending an option type argument after the 
 non-option argument.
 
 
  That would exactly account for the failure when we call 
 initdb foo 
  but not initdb -D foo.
 
  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.)

//Magnus

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster