Shouldn't this fix be upstreamed to libgit2? Kind of sketched about us maintaining a separate libgit2 that differs from published versions when we update.

@tbranyen There is an outstanding PR that includes this fix (libgit2/libgit2#3463) - this is the most critical part of that PR as it fixes a segfault. It is also probably the safest part - it only changes a malloc to a calloc.

The idea behind this PR was to get the segfault fixed in NodeGit until the upstream PR gets merged and released.

Definitely not interested in maintaining a forked version of libgit2. I'd say make another PR to libgit2/libgit2#maint/v0.23 so we can expedite getting a released version of this in.

However, until that lands I'm ok with using a fork as a temporary work around.

@tbranyen you cool with me merging this?

What is really fixing the problem? Changing malloc to calloc? That's fishy a bit.

@saper There is an explanation in libgit2/libgit2#3458 (comment)... it breaks down like this:

When we do entry->committer = git__malloc(sizeof(git_signature));, using malloc will not initialize the contents pointed to by entry->committer in any way, so the values of entry->committer->name, entry->committer->email etc. are indeterminate (that is, the values will be whatever happened to be in the memory block when it was allocated). If git_oid_fromstrn returns an error, we then immediately go to git_reflog_entry__free(entry), which in turn does git__free(entry->committer->name) and git__free(entry->committer->email). Because entry->committer->name and entry->committer->email are indeterminate, the free likely segfaults (in the included test, for me it would always segfault - in general, it will either do nothing if the value happens to be 0, free something it shouldn't free if the value happens to be a valid allocated pointer, or segfault / do some other kind of undefined behavior).

When using calloc instead of malloc, entry->committer = git__calloc(1, sizeof(git_signature)); will initialize the contents pointed to by entry->committer to all 0s. In the same scenario as above, when we get to git__free(entry->committer->name) and git__free(entry->committer->email), the free will do nothing because entry->committer->name and entry->committer->email are 0. That happens to be the correct behavior in this case.

I'm going to go ahead and merge this. We'll change back to using libgi2 proper after libgit2/libgit2#3458 is merged and released.