Skip to content

feat(rust): implement rust sled demo - #450

Merged
xushiwei merged 6 commits into
xgo-dev:mainfrom
hackerchai:feature/rust-sled
Jul 9, 2024
Merged

feat(rust): implement rust sled demo#450
xushiwei merged 6 commits into
xgo-dev:mainfrom
hackerchai:feature/rust-sled

Conversation

@hackerchai

Copy link
Copy Markdown
Contributor

Signed-off-by: hackerchai i@hackerchai.com

@xushiwei

xushiwei commented Jul 6, 2024

Copy link
Copy Markdown
Contributor

please write a README file like this: https://github.com/goplus/llgo/blob/main/c/sqlite/README.md

@hackerchai
hackerchai force-pushed the feature/rust-sled branch 2 times, most recently from ecfbe8a to d1e5db7 Compare July 8, 2024 07:48
Comment thread rust/sled/sled.go Outdated
Comment thread rust/sled/sled.go
Comment thread rust/sled/sled.go Outdated
Comment thread rust/sled/sled.go Outdated
}

//llgo:link (*SledConfig).OpenDB C.sled_open_db
func (conf *SledConfig) OpenDB() *SledDb {

@xushiwei xushiwei Jul 8, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's not common to add OpenDB in a Config type.

//go:linkname Open C.sled_open_db
func Open(conf *Config) *DB

To be confirmed: how sled_open_db indicates an open error?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's not common to add OpenDB in a Config type.

//go:linkname Open C.sled_open_db
func Open(conf *Config) *DB

To be confirmed: how sled_open_db indicates an open error?

In the implementation of the C wrapper in Sled, it directly calls unwrap to throw an error when there is an open error. For this C wrapper implementation, it indeed doesn't feel elegant. I think returning a null pointer for an open error might be a more elegant handling method.

/// Open a sled lock-free log-structured tree. Consumes the passed-in config.
#[no_mangle]
pub unsafe extern "C" fn sled_open_db(config: *mut Config) -> *mut Db {
    let config = Box::from_raw(config);
    Box::into_raw(Box::new(config.open().unwrap()))
}

When I call this function in C and pass in a null pointer, it directly throws an error:
Full Demo https://github.com/luoliwoshang/RustTest/blob/main/sled/test/test.c

 printf("Opening database...\n");

free(config); // mock error

void *db = sled_open_db(config);
printf("Database opened: %p\n", db);
if (db == NULL)
{
    printf("Failed to open database\n");
    sled_free_config(config);
    return 1;
}
./test
Creating config...
Config created: 0x6000031e4010
Setting path...
Path set, new config: 0x6000031e4020
Opening database...
[1]    6955 segmentation fault  ./test

@codecov

codecov Bot commented Jul 8, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.51%. Comparing base (794df59) to head (2b19513).
Report is 11 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #450   +/-   ##
=======================================
  Coverage   97.51%   97.51%           
=======================================
  Files          19       19           
  Lines        4420     4420           
=======================================
  Hits         4310     4310           
  Misses         94       94           
  Partials       16       16           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: hackerchai <i@hackerchai.com>
Signed-off-by: hackerchai <i@hackerchai.com>
@hackerchai
hackerchai force-pushed the feature/rust-sled branch from d1e5db7 to a36d5b6 Compare July 9, 2024 01:27
@xushiwei
xushiwei merged commit dbe13fe into xgo-dev:main Jul 9, 2024
@hackerchai
hackerchai deleted the feature/rust-sled branch August 1, 2024 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants