On Mon, 2007-07-30 at 21:43 -0700, Junio C Hamano wrote:
> Kristian Høgsberg <krh@xxxxxxxxxx> writes:
>
> > @@ -85,40 +129,20 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
> > parents++;
> > }
> >
> > - /* Not having i18n.commitencoding is the same as having utf-8 */
> > - encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);
> > + buffer = NULL;
> > + if (read_fd(0, &buffer, &len))
> > + die("Could not read commit message from standard input");
> >
> > - strbuf_init(&sb);
> > - strbuf_printf(&sb, "tree %s\n", sha1_to_hex(tree_sha1));
> > + commit_sha1 = create_commit(tree_sha1,
> > + parent_sha1, parents,
> > + xstrdup(git_author_info(1)),
> > + xstrdup(git_committer_info(1)),
> > + buffer, len);
>
> Hmph, the series was so nice so far, but here we have a few new
> leak, presumably so small per process invocation that we do not
> care about?
There's number of buffers that don't get freed: the strbuf, the commit
message buffer, and the strdup'ed author and committer info. All the
leaks are not critical since the process exits immediately. As for the
strbuf leak, I was thinking about renaming strbuf_begin to strbuf_reset
and making it public[1], which will then be used for freeing up strbuf
memory. The message buffer leak should be fixed by adding a
strbuf_read_fd() that just reads it straight into the strbuf. The
xstrdup's are necessary because fmt_ident uses a static buffer (thanks,
test case :). We could add rotating static buffers for fmt_ident like
git_path and avoid the strdups, but again, the leaks are not critical.
Kristian
[1] strbuf_begin() is a good name the way it's used in strbuf.c where
it's balanced by strbuf_end(), but as a general purpose reset function
it's better name strbuf_reset(), I think
-
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
