On Fri, 2016-01-15 at 14:33 +0100, Thomas Gummerer wrote:
> [I don't know too much about the refs code, but was still interested
> in this patch series, so below are the few things that I noticed
> while
> reading through it.]
Thanks for taking a look.
> On 01/14, David Turner wrote:
> > +exists. The header has the same format as an ordinary reflog, but
> > with
> > +a timeztamp of all zeros and an empty value.
>
> s/timeztamp/timestamp/
Fixed, thanks.
> > +test "$(git config extensions.refstorage)" = "lmdb" && die "git
> > -new-workdir is incompatible with the refs lmdb storage"
> > +
>
> Is it expected that other potential ref backends are compatible with
> git-new-workdir? Otherwise I think it would make more sense to
> whitelist the files backend here instead of blacklisting the lmdb
> backend, so we don't risk forgetting about this when adding another
> backend.
I no longer remember what the exact issue was, but we can be safe and
restrict git-new-workdir to the files backend (since it looks likely
that worktrees will replace it anyway).
> > case "$git_dir" in
> > .git)
> > git_dir="$orig_git/.git"
>
> [...]
>
> > +static int lmdb_init_db(struct strbuf *err, int shared)
> > +{
> > + /*
> > + * To create a db, all we need to do is make a directory
> > for
> > + * it to live in; lmdb will do the rest.
> > + */
> > +
> > + if (!db_path)
> > + db_path =
> > xstrdup(real_path(get_refdb_path(get_git_common_dir())));
>
> I think we're leaking some memory from get_refdb_path() here.
> get_refdb_path() uses strbuf_detach(), which according to its
> docstring
> makes its caller take care of the memory of the returned string.
> real_path() then uses strbuf_addstr() to add the string to its
> internal strbuf, but leaves the string we get from get_refdb_path()
> alone, so it leaks.
That only happens once per run, since db_path is set afterwards, but
I'll change get_refdb_path() to return a pointer to a statically
allocated string (as many similar funcs do). That will be easier to
read.
--
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