octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: RE: Re: New function proposal


From: John W. Eaton
Subject: RE: RE: Re: New function proposal
Date: Tue, 13 Feb 2007 13:22:09 -0500

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.

  +   if (! CreatePipe (&childRead, &parentWrite, NULL, 0) ||

Is NULL necessary?  Won't 0 work just as well in C++ code with proper
prototypes?

  +     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.

  +                       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?

Will someone please test the POSIX code and make sure that it works?

Thanks,

jwe


reply via email to

[Prev in Thread] Current Thread [Next in Thread]