Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for custom allocator #1100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add support for custom allocator #1100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions libgit2-sys/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,13 @@ pub struct git_writestream {
pub free: Option<extern "C" fn(*mut git_writestream)>,
}

#[repr(C)]
pub struct git_allocator {
pub gmalloc: *const extern "C" fn(size_t, *const c_char, c_int) -> *mut c_void,
pub grealloc: *const extern "C" fn(*mut c_void, size_t, *const c_char, c_int) -> *mut c_void,
pub gfree: *const extern "C" fn(*mut c_void),
}

git_enum! {
pub enum git_attr_value_t {
GIT_ATTR_VALUE_UNSPECIFIED = 0,
Expand Down
59 changes: 59 additions & 0 deletions src/opts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,29 @@ where
Ok(())
}

/// Set the memory allocator to a different memory allocator.
///
/// This allocator will then be used to make all memory allocations for
/// libgit2 operations. If the given `allocator` is None, then the
/// system default will be restored.
pub unsafe fn set_allocator(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For an unsafe fn, we'll need to put a comment here for callers to know how to use it safely

Copy link
Member

@weihanglo weihanglo Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, we don't provide this unsafe binding. People can use libgit2-sys directly at their own risks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's a fair point. Would it be better to take an std::alloc::Allocator as an input?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, though that is not what I was looked at. I was more on something like, could I call it multiple times? Could I reset after I allocated something in a custom allocator?

reset_search_path has this comment"

This function is unsafe as it mutates the global state but cannot guarantee
thread-safety. It needs to be externally synchronized with calls to access
the global state.

Do we have any considerations on the unsafety of this function?

gmalloc: *const extern "C" fn(libc::size_t, *const core::ffi::c_char, core::ffi::c_int) -> *mut core::ffi::c_void,
grealloc: *const extern "C" fn(*mut core::ffi::c_void, libc::size_t, *const core::ffi::c_char, core::ffi::c_int) -> *mut core::ffi::c_void,
gfree: *const extern "C" fn(*mut core::ffi::c_void),
) -> Result<(), Error> {
crate::init();
let allocator = raw::git_allocator{
gmalloc: gmalloc,
gfree: gfree,
grealloc: grealloc,
};
try_call!(raw::git_libgit2_opts(
raw::GIT_OPT_SET_ALLOCATOR as libc::c_int,
&allocator as *const raw::git_allocator
));
Ok(())
}

/// Reset the search path for a given level of config data to the default
/// (generally based on environment variables).
///
Expand Down Expand Up @@ -416,6 +439,8 @@ pub unsafe fn set_server_timeout_in_milliseconds(timeout: libc::c_int) -> Result

#[cfg(test)]
mod test {
use crate::test::repo_init;

use super::*;

#[test]
Expand Down Expand Up @@ -447,6 +472,40 @@ mod test {
}
}

static mut ALLOC_CALLED: bool = false;
static mut FREE_CALLED: bool = false;

#[test]
fn custom_allocator() {
unsafe {
extern "C" fn gmalloc(size: libc::size_t, _: *const core::ffi::c_char, _: core::ffi::c_int) -> *mut core::ffi::c_void {
unsafe {
ALLOC_CALLED = true;
libc::malloc(size)
}
}
extern "C" fn grealloc(ptr: *mut core::ffi::c_void, size: libc::size_t, _: *const core::ffi::c_char, _: core::ffi::c_int) -> *mut core::ffi::c_void {
unsafe {
ALLOC_CALLED = true;
libc::realloc(ptr, size)
}
}
extern "C" fn gfree(ptr: *mut core::ffi::c_void) {
unsafe {
FREE_CALLED = true;
libc::free(ptr)
}
}
assert!(set_allocator(
gmalloc as *const extern "C" fn(libc::size_t, *const core::ffi::c_char, core::ffi::c_int) -> *mut core::ffi::c_void,
grealloc as *const extern "C" fn(*mut core::ffi::c_void, libc::size_t, *const core::ffi::c_char, core::ffi::c_int) -> *mut core::ffi::c_void,
gfree as *const extern "C" fn(*mut core::ffi::c_void),
).is_ok());
repo_init();
assert!(ALLOC_CALLED);
assert!(FREE_CALLED);}
}

#[test]
fn server_connect_timeout() {
unsafe {
Expand Down
Loading