GNOME Bugzilla – Bug 702412
Empty tree can also be null
Last modified: 2019-02-22 03:52:08 UTC
Created attachment 246981 [details] [review] Fix for trees can also be NULL "You can pass NULL to indicate an empty tree, although it is an error to pass NULL for both the old_tree and new_tree." [1] The code in libgit2-glib/ggit-diff.c indicated that g_return_val_if_fail (GGIT_IS_TREE (old_tree), NULL); g_return_val_if_fail (GGIT_IS_TREE (new_tree), NULL); ...trees should not be NULL hence the bug 702259 [1] http://libgit2.github.com/libgit2/#HEAD/group/diff/git_diff_tree_to_tree Patch attached.
I'd keep the checks, but explicitly check also for NULL.
something like: g_return_val_if_fail (old_tree == NULL || GGIT_IS_TREE (old_tree), NULL); g_return_val_if_fail (new_tree == NULL || GGIT_IS_TREE (new_tree), NULL); g_return_val_if_fail (old_tree != NULL || new_tree != NULL, NULL);
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.
Can I have the commit ID please? I am unable to find it in master.
Found it. Committed to master in 8a95d39 Available at https://git.gnome.org/browse/libgit2-glib/commit/?id=8a95d39e9098af4a4d18f47abfc4420a596b4acc
From libgit2 docs: "You can pass NULL to indicate an empty tree, although it is an error to pass NULL for both the old_tree and new_tree." I think you should check (like I suggested above) that either old_tree or new_tree should be non NULL.
> I think you should check (like I suggested above) that either old_tree or > new_tree should be non NULL. Err, nacho already fixed and committed this to wip/development branch. So now I am working on the bug 702259.
I'm just pointing out that the fix isn't 100% correct.
Thanks for the review, didn't realized about that part of the docs.