fix custom_key_cmp_wrapper being able to unwind to C code (ub)

[antonilol]

custom_key_cmp_wrapper may not panic because it is called from C code that does not understand rust panics. If it does, that is undefined behavior.

custom_key_cmp_wrapper will now abort if the Comparator implementation panics. I also removed the safety comments on compare and compare_elem (the safety comments on the safe trait is what got me to discover this UB)

Review note:

match ord {
    Ordering::Less => -1,
    Ordering::Equal => 0,
    Ordering::Greater => 1,
}

is equivalent to ord as i32

[Kerollmops]

Thank you very much @antonilol to make the crate much more safe than it is already!

BTW what do you use heed for? 😊

[Kerollmops]

Why have you added unsafe to this function? Isn't it safe now?

[antonilol]

It still dereferences raw pointers. Should I add a /// # Safety doc comment with some explanation? It is a private function though.

[Kerollmops]

Ho! Yeah! You're right. It's a good idea to add a safety note.

[antonilol]

Added the safety note

[antonilol]

BTW what do you use heed for? 😊

I am currently experimenting with changing the database backend of electrs, it currently uses RocksDB but that takes so long to compile.

[Kerollmops]

Thank you very much for the fixes! Merging 🙏

[Kerollmops]

I am currently experimenting with changing the database backend of electrs, it currently uses RocksDB but that takes so long to compile.

Nice, @antonilol 👌 So, you switch not for the LMDB performances or functionalities but more to reduce compilation times?

[antonilol]

Nice, @antonilol 👌 So, you switch not for the LMDB performances or functionalities but more to reduce compilation times?

Mainly compile time, but the memory mapped nature of LMDB should also speed up runtime by a bit. Important functionality is about the same, talking about get, put, delete, and iterators.