| Message ID | 5f2e816cf6359725f2a86ce1d08e5e272fba4dac.1723054623.git.steadmon@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce cgit-rs, a Rust wrapper around libgit.a | expand |
On Wed, Aug 07, 2024 at 11:21:27AM -0700, Josh Steadmon wrote: > Non-C external consumers of libgit.a have to redefine the `repository` > object in their own language if they want to call > initialize_repository() to ensure memory for the object is allocated > correctly. This is not ideal for external consumers that have no need > for the entire `the_repository` object but need to call other functions > from an initialized repository. Therefore, add a friendly > initialize_repository() wrapper without a `the_repository` pointer. Technically speaking, you don't really need this. You can define `repository` as an opaque type in Rust: ``` #[allow(non_camel_case_types)] #[repr(C)] pub struct repository([u8; 0]); ``` And define `the_repository` as an extern symbol: ``` extern "C" { pub static mut the_repository: *mut repository; } ``` Mike
On 2024.08.08 07:52, Mike Hommey wrote: > On Wed, Aug 07, 2024 at 11:21:27AM -0700, Josh Steadmon wrote: > > Non-C external consumers of libgit.a have to redefine the `repository` > > object in their own language if they want to call > > initialize_repository() to ensure memory for the object is allocated > > correctly. This is not ideal for external consumers that have no need > > for the entire `the_repository` object but need to call other functions > > from an initialized repository. Therefore, add a friendly > > initialize_repository() wrapper without a `the_repository` pointer. > > Technically speaking, you don't really need this. > > You can define `repository` as an opaque type in Rust: > ``` > #[allow(non_camel_case_types)] > #[repr(C)] > pub struct repository([u8; 0]); > ``` > > And define `the_repository` as an extern symbol: > ``` > extern "C" { > pub static mut the_repository: *mut repository; > } > ``` > > Mike I've actually already done a refactor for V2 that will avoid using this patch entirely, but thank you for the pointer. We do something similar to opaquely wrap configset pointers in a later patch (we use an empty enum there, I'm not sure whether that approach or a zero-size array is preferred).
On Wed, Aug 07, 2024 at 04:23:05PM -0700, Josh Steadmon wrote: > On 2024.08.08 07:52, Mike Hommey wrote: > > On Wed, Aug 07, 2024 at 11:21:27AM -0700, Josh Steadmon wrote: > > > Non-C external consumers of libgit.a have to redefine the `repository` > > > object in their own language if they want to call > > > initialize_repository() to ensure memory for the object is allocated > > > correctly. This is not ideal for external consumers that have no need > > > for the entire `the_repository` object but need to call other functions > > > from an initialized repository. Therefore, add a friendly > > > initialize_repository() wrapper without a `the_repository` pointer. > > > > Technically speaking, you don't really need this. > > > > You can define `repository` as an opaque type in Rust: > > ``` > > #[allow(non_camel_case_types)] > > #[repr(C)] > > pub struct repository([u8; 0]); > > ``` > > > > And define `the_repository` as an extern symbol: > > ``` > > extern "C" { > > pub static mut the_repository: *mut repository; > > } > > ``` > > > > Mike > > I've actually already done a refactor for V2 that will avoid using this > patch entirely, but thank you for the pointer. We do something similar > to opaquely wrap configset pointers in a later patch (we use an empty > enum there, I'm not sure whether that approach or a zero-size array is > preferred). An empty enum is a never type, I wouldn't recommend using it as an opaque wrapper. It will likely lead to the compiler doing bad things. https://rust-lang.github.io/never-type-initiative/RFC.html `#[repr(C)]` and `[u8; 0]` are recommended by the nomicon. https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs (the PhantomPinned wasn't there last time I saw that page) Mike
On 2024.08.08 08:29, Mike Hommey wrote: > On Wed, Aug 07, 2024 at 04:23:05PM -0700, Josh Steadmon wrote: > > On 2024.08.08 07:52, Mike Hommey wrote: > > > On Wed, Aug 07, 2024 at 11:21:27AM -0700, Josh Steadmon wrote: > > > > Non-C external consumers of libgit.a have to redefine the `repository` > > > > object in their own language if they want to call > > > > initialize_repository() to ensure memory for the object is allocated > > > > correctly. This is not ideal for external consumers that have no need > > > > for the entire `the_repository` object but need to call other functions > > > > from an initialized repository. Therefore, add a friendly > > > > initialize_repository() wrapper without a `the_repository` pointer. > > > > > > Technically speaking, you don't really need this. > > > > > > You can define `repository` as an opaque type in Rust: > > > ``` > > > #[allow(non_camel_case_types)] > > > #[repr(C)] > > > pub struct repository([u8; 0]); > > > ``` > > > > > > And define `the_repository` as an extern symbol: > > > ``` > > > extern "C" { > > > pub static mut the_repository: *mut repository; > > > } > > > ``` > > > > > > Mike > > > > I've actually already done a refactor for V2 that will avoid using this > > patch entirely, but thank you for the pointer. We do something similar > > to opaquely wrap configset pointers in a later patch (we use an empty > > enum there, I'm not sure whether that approach or a zero-size array is > > preferred). > > An empty enum is a never type, I wouldn't recommend using it as an opaque > wrapper. It will likely lead to the compiler doing bad things. > https://rust-lang.github.io/never-type-initiative/RFC.html > > `#[repr(C)]` and `[u8; 0]` are recommended by the nomicon. > https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs > > (the PhantomPinned wasn't there last time I saw that page) > > Mike Fixed in V2 (for ConfigSet). Thanks!
diff --git a/repository.c b/repository.c index 9825a30899..15cb9ee735 100644 --- a/repository.c +++ b/repository.c @@ -81,6 +81,15 @@ void initialize_repository(struct repository *repo) set_default_hash_algo(repo); } +/* + * For non-C external consumers of libgit.a that do not need access + * to the entire `the_repository` object. + */ +void initialize_the_repository(void) +{ + initialize_repository(the_repository); +} + static void expand_base_dir(char **out, const char *in, const char *base_dir, const char *def_in) { diff --git a/repository.h b/repository.h index af6ea0a62c..65c9158866 100644 --- a/repository.h +++ b/repository.h @@ -227,6 +227,7 @@ void repo_set_compat_hash_algo(struct repository *repo, int compat_algo); void repo_set_ref_storage_format(struct repository *repo, enum ref_storage_format format); void initialize_repository(struct repository *repo); +void initialize_the_repository(void); RESULT_MUST_BE_USED int repo_init(struct repository *r, const char *gitdir, const char *worktree);