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 <[email protected] > <mailto:[email protected]>> 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-throw-suspiscion-on-potential-double-free-problem > > <https://pharo.fogbugz.com/f/cases/20655/vm-crash-in-libgit-git_tree_free-throw-suspiscion-on-potential-double-free-problem> > > 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. > > >
