Re: Fix initdb for path with whitespace and at char

On 05/01/2014 07:55 AM, Amit Kapila wrote:
> On Wed, Apr 30, 2014 at 4:01 PM, Heikki Linnakangas
> <hlinnakangas@vmware.com> wrote:
>> I committed the non-invasive fixes to backbranches (and master too, just to
>> keep it in sync), but the attached is what I came up with for master.
>>
>> There are a couple of places in the code where we have #ifdef WIN32 code
>> that uses CreateProcess with "CMD /C ..." directly.
>
> 1. Do we need similar handling for CreatePipe case where it directly uses
> executable path such as in function pipe_read_line()?
> Currently the caller of pipe_read_line() calls canonicalize_path() to change
> win32 specific path, is that sufficient or do we need SYSTEMQUOTE type
> of handling for it.

No, SYSTEMQUOTE style handling is not required with CreateProcess. 
find_other_exec, which is the only caller of pipe_read_line, adds one 
pair of double-quotes around the executable's path, which is enough for 
CreateProcess.

> 2.
> system.c
> #include <assert.h>
> Do we really need inclusion of assert.h or this is for future use?

You're right, that's not needed.

> 3. One more observation is that currently install.bat doesn't work
> for such paths:
> install.bat "e:\PostgreSQL\master\install 1\ins@1"
> 1\ins@1""=="" was unexpected at this time.

Yeah, I noticed that. I haven't looked into what it would take to fix 
that, but for now you can just install to a path that doesn't contain 
whitespace, and move it from there. install.bat is only used by 
developers / packagers, so that's not a big deal.

> 4. Similar to Andrew, I also could not reproduce this problem on my
> Windows system (Windows 7 64 bit)
> e:\>"e:\PostgreSQL\master\install 1\ins@1\bin\initdb.exe" -D "e:
> \PostgreSQL\master\Data 1"
> e:\>"e:\PostgreSQL\master\install 1\ins@1\bin\pg_ctl.exe" start -D "e:
> \PostgreSQL\master\Data 1"
>
> Above commands work fine.

Hmm, I'm also testing on 64-bit Windows 7, and it failed for me. Note 
that I already committed the narrow fix for initdb - which I also 
backpatched - to master, so to reproduce you'll need to revert that 
locally (commit 503de546).

I fixed the issues with malloc that Tom pointed out and committed the 
wrapper functions to git master. But please let me know if there's still 
a problem with it.

- Heikki