On Tue, Jan 08, 2002 at 01:06:08PM -0600, Karl Fogel wrote:
> Ooooh, that's bad. Suspect something wrong with the error checking in
> this part of libsvn_wc/get_editor.c, but not sure. Let us know what
> you find, thanks for looking into this...
>
> [...]
> apr_err = apr_proc_create (&diff_proc,
> SVN_CLIENT_DIFF,
> diff_args,
> NULL,
> diffproc_attr,
> fb->pool);
> if (! APR_STATUS_IS_SUCCESS (apr_err))
> return svn_error_createf
> (apr_err, 0, NULL, fb->pool,
> "close_file: error starting diff process");
>
> /* Wait for the diff command to finish. */
> apr_err = apr_proc_wait (&diff_proc, NULL, NULL, APR_WAIT);
> if (APR_STATUS_IS_CHILD_NOTDONE (apr_err))
> return svn_error_createf
> (apr_err, 0, NULL, fb->pool,
> "close_file: error waiting for diff process");
> [...]
ok, the problem is in this code, but the version of it in
svn_io_run_cmd (for this case, although i imagine it will fail in the
same manner elsewhere).
when we run apr_proc_create, it forks off a process, which suceeds,
so that suceeds just fine. that process tries to exec diff, which
fails for obvious reasons. it then does an 'exit(-1)'. (this occurs
in apr/threadproc/unix/proc.c for those of you playing along at home)
when we do the apr_proc_wait, the APR_STATUS_OS_CHILD_NOTDONE will
return false, because the child is done, it exited with a -1. now we
don't always call apr_proc_wait with an exitcode parameter, but if we
did (as we do in svn_io_run_cmd, but not in get_editor.c), we would
recieve from apr the output of this line:
*exitcode = WEXITSTATUS(exit_int);
which turns out to be 255, which i imagine it some kind of error code
for this sort of thing.
now the question is what to do about it. i have a patch that will
just handle this for SVN_CLIENT_DIFF. we can just check that diff
exited with either 0, 1, or 2 (the only exit codes it is supposed to
use, according to it's man page), and if it exits with something else,
we know there's a problem and we error out.
that seems kind of ugly though, and it seems like there must be some
kind of facility in apr for checking for this condition, or if there
isn't, there should be.
unfortunately, i can't find it, and if it isn't there now i'm not sure
how one should implement it.
in any case, i will be following an issue about this, as soon as i take
a break and eat some dinner.
-garrett
--
garrett rooney Unix was not designed to stop you from
rooneg@electricjellyfish.net doing stupid things, because that would
http://electricjellyfish.net/ stop you from doing clever things.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:55 2006