on 4/12/05 12:35 AM, Brian Candler <B.Ca...@pobox.com> wrote:
On Mon, Apr 11, 2005 at 06:43:37PM -0700, Kurt Bigler wrote:
If there were any problem with the executable not being there, I'd off-hand
have expected this error instead:
ispell_run: fork_ispell failed
The fork was successful; it was the exec that failed.
Yes, fork_ispell also takes care of the exec. My meaning was that the
function fork_ispell was not really successful, although I suppose that's a
matter of definition, and I suppose there currently is none.
But if it's achievable I think fork_ispell should return non-zero if any of
the fork or exec functions fail, so that the "fork_ispell failed" error
could be given in any of these cases.
The code does say:
execv(ISPELL, (char **)args);
perror("fork_ispell: execv " ISPELL);
But perhaps the stderr descriptor isn't available by then. I can't see why
through, from a quick look at the code.
Yes, this is not the message that appeared in the log. The one that did
(ispell_run: incomplete result from ispell) is output by ispell_run.
Or if you *did* get this message, perhaps it should be changed to
perror("fork_ispell: failed to execv " ISPELL);
Well I didn't get that message but it is certainly an improvement. But it
looks like something's not quite right and stderr is not behaving according
to what *appears* to be the design.
I can't see the stderr problem yet.
But I do see one thing that might be a potential improvement. Right now
fork_ispell is not trying to catch the status values returned by any of the
forked processes. My suggestion would be to start by changing wait4pid,
currently:
static void wait4pid()
{
int waitstat;
if (prevsighandler == SIG_DFL)
while ( wait(&waitstat) != ispell_pid)
;
}
to this:
static int wait4pid()
{
int waitstat;
if (prevsighandler == SIG_DFL)
{
while ( wait(&waitstat) != ispell_pid)
;
}
else
waitstat = ???; /*not sure what to do here*/
return waitstat;
}
and then make all the relevant status values passed to exit unique within
fork_ispell, and then have fork_ispell utilize the value returned by
wait4pid, perhaps as the function return value at the bottom of the
function, instead of 0.
There is still something missing from this picture, because there are two
levels of fork, but only one wait on the single ispell_pid. But that's
enough said from my side for starters.
-Kurt