[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: New function proposal
From: |
Michael Goffioul |
Subject: |
Re: New function proposal |
Date: |
Wed, 14 Feb 2007 14:10:41 +0100 |
User-agent: |
Thunderbird 1.5.0.9 (Windows/20061207) |
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.
- New function proposal, michael . goffioul, 2007/02/12
- RE: Re: New function proposal, michael . goffioul, 2007/02/13
- Re: New function proposal, David Bateman, 2007/02/13
- Re: New function proposal, John W. Eaton, 2007/02/13
- Re: New function proposal, Paul Kienzle, 2007/02/13
- Re: New function proposal, John W. Eaton, 2007/02/14
- Re: New function proposal, Michael Goffioul, 2007/02/15
- Re: New function proposal, David Bateman, 2007/02/15
- Re: New function proposal, John W. Eaton, 2007/02/15
- Re: New function proposal, David Bateman, 2007/02/15