On Tue, Mar 01, 2016 at 03:36:26PM -0800, Junio C Hamano wrote: > This will be necessary when we start reading from a split bundle > where the header and the thin-pack data live in different files. > > The in-core bundle header will read from a file that has the header, > and will record the path to that file. We would find the name of > the file that hosts the thin-pack data from the header, and we would > take that name as relative to the file we read the header from. Neat. I'm hoping this means you're working on split bundles. :) > diff --git a/builtin/bundle.c b/builtin/bundle.c > index 4883a43..e63388d 100644 > --- a/builtin/bundle.c > +++ b/builtin/bundle.c > @@ -36,8 +36,9 @@ int cmd_bundle(int argc, const char **argv, const char *prefix) > } > > memset(&header, 0, sizeof(header)); > - if (strcmp(cmd, "create") && (bundle_fd = > - read_bundle_header(bundle_file, &header)) < 0) > + header.bundle_file = bundle_file; What are the memory ownership rules for header.bundle_file? Here you assign from either an argv parameter or a stack buffer, and here... > @@ -112,6 +111,8 @@ void release_bundle_header(struct bundle_header *header) > for (i = 0; i < header->references.nr; i++) > free(header->references.list[i].name); > free(header->references.list); > + > + free((void *)header->bundle_file); > } You free it. The call in get_refs_from_bundle does do an xstrdup(). Should we have: void init_bundle_header(struct bundle_header *header, const char *file) { memset(header, 0, sizeof(*header)); header.bundle_file = xstrdup(file); } to abstract the whole procedure? -Peff -- 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