Juergen Spitzmueller wrote:

> Georg Baum wrote:
>> Why not add a
>>
>> cmd_ret const Forkedcall::startscript(string const & what)
>>
>> method that replaces cmd_ret const RunCommand(string const & cmd)
>> and implements the piping on top of the forked controller stuff? In
>> fact, the piping is already implemented in src/ispell.C, so it is
>> probably a matter of some code shuffling around only.
> 
> Do you want to try it? My knowledge of piping/forks is clearly
> smaller than yours.
> BTW I figured that RunCommand is the only place where we use
> popen/pclose now (AFAICS). RunCommand is used by findtexfile (which
> is used by insetbibtex and LaTeX.C to search for bib files) and by
> buffer.C (to invoke lyx2lyx).

Juergen, as you say, I'm sure that this error has been 
triggered by the use of the SIGCHLD handler in forkedcontr.C. 
However, your statement that popen is now only used in 
RunCommand is also true. We could probably get rid of all calls 
to RunCommand (see below). However, for now, I think that we should
just fix the bug.

Could you see if adding this to RunCommand fixes the problem?

cmd_ret const RunCommand(string const & cmd)
{
        sigset_t newMask, oldMask;
        sigemptyset(&oldMask);
        sigemptyset(&newMask);
        sigaddset(&newMask, SIGCHLD);

        // Block the SIGCHLD signal.
        sigprocmask(SIG_BLOCK, &newMask, &oldMask);

        // Existing code in RunCommand goes here...

        // Unblock the SIGCHLD signal and restore the old mask.
        sigprocmask(SIG_SETMASK, &oldMask, 0);
}






$ grep -r popen src | grep -v ChangeLog
src/support/filetools.C:        // One question is if we should use popen or
src/support/filetools.C:        // create our own popen based on fork, exec, pipe
src/support/filetools.C:        FILE * inf = ::popen(cmd.c_str(), 
os::popen_read_mode());
src/support/filetools.C:        // (Claus Hentschel) Check if popen was succesful ;-)
src/support/os.h:// returns a string suitable to be passed to popen when
src/support/os.h:// same for popen().
src/support/os.h:       char const * popen_read_mode();
src/support/os_os2.C:// returns a string suitable to be passed to popen when
src/support/os_os2.C:char const * popen_read_mode()
src/support/os_unix.C:char const * popen_read_mode()
src/support/os_win32.C:// returns a string suitable to be passed to popen when
src/support/os_win32.C:char const * popen_read_mode()

It looks to me as if 'popen_read_mode' can also be removed.

So the question becomes, where is RunCommand invoked?

Buffer::readFile:
                string command = LibFileSearch("lyx2lyx", "lyx2lyx");
                command += " -t"
                        + tostr(LYX_FORMAT)
                        + " -o " + tmpfile + ' '
                        + QuoteName(filename);
                cmd_ret const ret = RunCommand(command);
This instance can certainly be replaced by a call to
        ForkedCall call;
        cmd_ret const ret = 
                call.startscript(ForkedCall::Wait, command);

In math_extern.C
        string captureOutput(string const & cmd, string const & data)
        {
                string command =  "echo '" + data + "' | " + cmd;
                lyxerr << "calling: " << command << endl;
                cmd_ret const ret = RunCommand(command);
                return ret.second;
        }

Hmmmm. This is more complex since it involves a pipe. 
However, as Georg says, we handle something similar in ispell.C.

filetools.C, findtexfile:
Shrug. This could be replaced with a call to ForkedCall::startscript
similar to Buffer::readFile.


-- 
Angus

Reply via email to