Unsafe code review request - new crate concurrent-interner

code review
#1

Hey folks :wave:, I'm looking for someone with expertise with unsafe code to review my new crate. GitHub - typesanitizer/concurrent-interner: A string interner that is safe to use from multiple threads

I've run the tests under MIRI and they pass (:tada:) after about 45 minutes (:cry:), which makes it untenable to run in CI. Happy to take feedback here or via GitHub issues. Thanks!

#2

Hi, concurrent-interner looks really interesting! I'll take a look at it over the next few days.

I'm far from expert, but I have done a bit of unsafe Rust code, so I'll let you know if anything jumps out at me.

One thing: there's a magic number '4192' in the code. Was it meant to be 4096?

1 Like
#3

Thanks, that's a typo, should be fixed in fix: 4K = 4096, not 4192 by typesanitizer · Pull Request #5 · typesanitizer/concurrent-interner · GitHub

#4

@typesanitizer Just FYI, I couldn't find any problems with this code. I don't know anything about panic-safety, though, so no idea if the code is OK in that respect.
Nice code, I found it very interesting to review!

1 Like
#5

Just a minor nitpick: u16::max_value() and usize::max_value() are (or will be) deprecated, so they should really be replaced with u16::MAX and usize::MAX.

#6

Thanks, fixed in refactor: Use MAX instead of max_value(). by typesanitizer · Pull Request #6 · typesanitizer/concurrent-interner · GitHub.

#7

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.