Re: New function proposal

[Michael Goffioul]

John W. Eaton a écrit :
> On 13-Feb-2007, address@hidden wrote:
>
> | Here's another variant, which moves popen2 into C++ and provides
> | Win32 implementation. Note that the UNIX version is not tested *at all*
> | (I don't even know if it compiles): I just tried to convert popen2.m in
> | C++, but it probably needs fixing.
>
> I suppose having popen2 and pipe as built-in functions is OK.
>
> I don't like that we are scattering system dependent (non POSIX) code
> all over Octave, so I think it would be better to collect all the
> Windows-specific code like this that is large enough to be a separate
> function and put it in a separate file.
>   

That' why I used oct-syscalls.cc. How do see this then? Can you make it 
clearer how you
would like to see it? oct-syscalls-win32.cc, oct-syscalls-posix.cc...?

>   +   if (! CreatePipe (&childRead, &parentWrite, NULL, 0) ||
>
> Is NULL necessary?  Won't 0 work just as well in C++ code with proper
> prototypes?
>   

It's a pointer, so reinterpret_cast<LPSECURITY_ATTRIBUTES>(0) or
(LPSECURITY_ATTRIBUTES)0 should be OK. I just found NULL easier
to write. I forgot it had to avoided in C++.

>   +     command += " \"" + args[k] + "\"";
>   +   if (! CreateProcess (NULL, const_cast<char*> (command.c_str()), NULL, NULL, TRUE, 
> 0, NULL, NULL, &si, &pi))
>
> Can CreateProcess write to the command string?  If not, then why isn't
> it declared const?  Rather than a const_cast, I think it would be
> better to copy the command you construct to a local buffer.
>   

The argument has type LPTSTR, which correspond to char*. The 
documentation specifies
this argument as in/out, but it's not clear what kind of output it is 
used for. I'll use a local
buffer.

>   +                       OCTAVE_LOCAL_BUFFER (char, c_msg, msg.length () + 1);
>   +                       strcpy (c_msg, msg.c_str ());
>   +                       error (c_msg);
>
> I don't see the need for the buffer here.  Doesn't
>
>   error (msg.c_str ())
>
> work here?
>   

I guess it does. This was just to be safe, as the memory location 
returned by
c_str() is temporary, isn't it? I should have switched my safety worries 
with
the previous item :-|

Michael.