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