On Mon, Sep 9, 2024 at 1:47 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
> >> +struct libgit_config_set *libgit_configset_alloc(void)
> >> +{
> >> + return git_configset_alloc();
> >> +}
> >
> > git_configset_alloc() returns "struct config_set *" while this thing
> > returns an incompatible pointer.
> >
> > Sent out an outdated version or something? This wouldn't have
> > passed even a compile test, I suspect.
>
> The "shim" layer should hide the details of interfacing to the git
> proper from callers, as well as it should hide the callers' from the
> git proper. So if you really want to hide "struct config_set" from
> your library callers, you may need to do something like the
> attached, perhaps? At least this does pass compilation test.
You're correct that this is supposed to be the shim layer, where
callers aren't supposed to be able to directly interface with the
config_set struct. From Rust's perspective, all pointers that come
back from C code are effectively void* so the naming doesn't make a
functional difference for our use case (and consequently these
warnings did not show up from the build.rs script nor did the tests
fail). However, I agree that the public interface should pass the
compilation test and your approach does that -- will reroll with those
changes and I believe that we should also fix the build.rs so that
warnings also show up during cargo build.
