yeah, but this is because LGitDiff, LGitRepository and LGitTree (and many others) are defined as FFIExternalObjects while they should be FFIOpaqueObjects. Then, the problem is that external objects are *already* pointers (they have arity 1). this are remaining of old nativeboost and we created the opaque objects to not need this âcheatingâ⦠just, I wanted to change them but is a lot of work and I didnât have the time. I agree it stinks. Esteban
On 8 Nov 2017, at 19:06, Nicolas Cellier <nicolas.cellier.aka.nice@gmail.com <mailto:nicolas.cellier.aka.nice@gmail.com>> wrote:
I was trying to inquire about my latest vm crash in libgit2
https://pharo.fogbugz.com/f/cases/20655/vm-crash-in-libgit-git_tree_free-thr... <https://pharo.fogbugz.com/f/cases/20655/vm-crash-in-libgit-git_tree_free-thr...>
when I found this bizarre prototype:
^ self callUnchecked: #(LGitReturnCodeEnum git_diff_tree_to_tree #(LGitDiff * diff , LGitRepository repo , LGitTree old_tree , LGitTree new_tree , LGitDiffOptions * opts)) options: #()
While the libgit2 signature differs in indirection levels: https://libgit2.github.com/libgit2/#v0.19.0/group/diff/git_diff_tree_to_tree <https://libgit2.github.com/libgit2/#v0.19.0/group/diff/git_diff_tree_to_tree>
int git_diff_tree_to_tree(git_diff_list **diff, git_repository *repo, git_tree *old_tree, git_tree *new_tree, const git_diff_options *opts);
The fact that it does not differs for opts, made me think of a bug... But no.
opts is allocated on external c heap with:
callUnchecked: #(LGitReturnCodeEnum git_diff_init_options(LGitDiffOptions * self, LGitOptionsVersionsEnum version))
int git_diff_init_options(git_diff_options *opts, unsigned int version);
What you see this time is that signatures match... So the LGitDiffOptions handle will be an ExternalAddress that will really point on a git_diff_options In other words, the handle is a git_diff_options *... IOW, our LGitDiffOptions object points to an external it_diff_options so it is what it promises to be.
For the other structures not so. We are lying on the level of indirection, so our LGitTree handle are not really a git_tree *. They are a git_tree **. (an ExternalAddress on a pointer).
As long as we lie consistently, it's OK, because it avoids too much packToArity: unpackToArity: dance.
That's the "beauty" of C pointers. We can cast them to whatever or pretend they are handle_for_anything * in the intermediate level, as long as original producer and final consumer agree on what it really points to.
But from code quality perspective, it really stinks. Anyone like me opening a page to get the exact signature of the function will be scratching head and loose precious time. Especially when tracking vm crash down.
I'm not sure how well it's documented, I presume it's a well known conscious hack from the original developpers, but such practice really ought to be discussed here.