On Samstag, 9. Januar 2010, Ilari Liusvaara wrote:
> 6 files changed, 282 insertions(+), 13 deletions(-)
IMHO, you are going completely overboard with your patch.
You check every single possible and unlikely error condition in the new code
that you introde, but ignore that there are quite a number of calls in the
existing code whose error conditions go checked.
And as a result, you "only" report errno values of some calls, and ignore that
there is at least one potential die() call hidden in execv_git_cmd(). IOW,
your code is not a catch-all and only an isolated solution.
I hope I can do better.
I developed this series on top of il/vcs-helper. But patches 1-3/4 are
actually an independent topic and also suitable to be merged into
jk/run-command-use-shell to implement improved DWIM whether a shell is
needed.
Patch 4/4 is to address your problem of weak error reporting with transport
helpers. I think I understand now what your problem is -- namely that
previously no error was reported if a transport helper program did not exist.
The reason for the missing error message is IMO a design weakness in the
protocol: It requires that the parent talks first - and by talking to an
early-died child process it dies from SIGPIPE. I experimented with
signal(SIGPIPE, SIG_IGN) to get around this, but it didn't work (the SIGPIPE
arrived anyway).
Anyway, patch 4/4 goes on top of il/vcs-helper after merging the topic
consisting of 1-3/4.
Ilari Liusvaara (1):
Improve error message when a transport helper was not found
Johannes Sixt (3):
start_command: report child process setup errors to the parent's
stderr
run-command: move wait_or_whine earlier
start_command: detect execvp failures early
Makefile | 1 +
run-command.c | 177
+++++++++++++++++++++++++++++++++++-------------
t/t0061-run-command.sh | 14 ++++
test-run-command.c | 35 ++++++++++
transport-helper.c | 14 +++-
5 files changed, 191 insertions(+), 50 deletions(-)
create mode 100755 t/t0061-run-command.sh
create mode 100644 test-run-command.c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html