On Sonntag, 13. September 2009, Junio C Hamano wrote: > Jim Meyering <jim@xxxxxxxxxxxx> writes: > > diff --git a/transport-helper.c b/transport-helper.c > > index f57e84c..0bbd014 100644 > > --- a/transport-helper.c > > +++ b/transport-helper.c > > @@ -49,6 +49,7 @@ static struct child_process *get_helper(struct > > transport *transport) if (!strcmp(buf.buf, "fetch")) > > data->fetch = 1; > > } > > + fclose (file); > > return data->helper; > > } > > > > @@ -88,6 +89,7 @@ static int fetch_with_fetch(struct transport > > *transport, if (strbuf_getline(&buf, file, '\n') == EOF) > > exit(128); /* child died, message supplied already */ > > } > > + fclose (file); > > return 0; > > } > > The callchain of fetch_with_fetch() looks like: > > fetch_with_fetch() > helper = get_helper(); > --> get_helper() > - start helper with start_command(); > - read from helper->out until it sees an empty line; > - break out of the loop; > <-- return helper > - file = xfdopen(helper->out) to get another FILE on the fd > - read the rest of the output from helper->out via file > > It seems to me that the fclose() in get_helper() will close the underlying > fd and would break the caller, no? > > I think "struct helper_data" should get a new FILE* field and once > somebody creates a FILE* out of its helper->out, that FILE* can be passed > around without a new xfdopen(). > > Or something like that. > > Who is responsible for closing the underlying helper->out fd in the > start_command() API, by the way? A pipe was requested by setting helper->out = -1 before the call to start_command(), and in such a case the caller must close the fd. -- Hannes -- 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