To make the code simpler, run_textconv lumps all of its
error checking into one conditional. However, the
short-circuit means that an error in reading will prevent us
from calling finish_command, leaving a zombie child.
Clean up properly after errors.
Based-on-work-by: Jeff King <peff@xxxxxxxx>
Signed-off-by: Johannes Sixt <j6t@xxxxxxxx>
---
Jeff,
> Yes, there are clever ways to make this shorter, but I think it is
> clearer just written out.
Thanks for your work, but I'm worried that in your version the close()
call is not before the finish_command (but that's really not _that_
important in this case). My version is perhaps not as easy to read, but
has a slightly better diff-stat.
Oh, I removed the error messages after start_command and finish_command,
because these two print one themselves; Junio, if you disagree with
this change, ditch my version and take Jeff's.
-- Hannes
diff.c | 17 +++++++++++------
1 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/diff.c b/diff.c
index dfdfa1a..b96faea 100644
--- a/diff.c
+++ b/diff.c
@@ -3874,6 +3874,7 @@ static char *run_textconv(const char *pgm, struct diff_filespec *spec,
const char **arg = argv;
struct child_process child;
struct strbuf buf = STRBUF_INIT;
+ int err = 0;
temp = prepare_temp_file(spec->path, spec);
*arg++ = pgm;
@@ -3884,16 +3885,20 @@ static char *run_textconv(const char *pgm, struct diff_filespec *spec,
child.use_shell = 1;
child.argv = argv;
child.out = -1;
- if (start_command(&child) != 0 ||
- strbuf_read(&buf, child.out, 0) < 0 ||
- finish_command(&child) != 0) {
- close(child.out);
- strbuf_release(&buf);
+ if (start_command(&child)) {
remove_tempfile();
- error("error running textconv command '%s'", pgm);
return NULL;
}
+
+ if (strbuf_read(&buf, child.out, 0) < 0)
+ err = error("error reading from textconv command '%s'", pgm);
close(child.out);
+
+ if (finish_command(&child) || err) {
+ strbuf_release(&buf);
+ remove_tempfile();
+ return NULL;
+ }
remove_tempfile();
return strbuf_detach(&buf, outsize);
--
1.7.0.2.250.ge5578
--
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