Pedantic: that's C++, not C. C doesn't have anything like using
, decltype
or nullptr
.
The original code uses fixed-width integer types. u32
and i32
aren't standard types, those would be uint32_t
and int32_t
, but these typedefs are also quite common in C++ code. In any case, translating those as c_uint
and c_int
would be incorrect. c_uint
corresonds to unsigned int
, which is a platform-dependent type with size mostly undefined by the standard. Similarly, c_int
should be used only when the original used int
.
That's a bit of an odd code. Why set initially to nullptr
and introduce the possibility of bugs, when you can assign the value directly? Also, why use decltype
when the type is explicitly known, and most importantly determined by the name of the linked dll function? Casting a function pointer to an incorrect type is UB, you should be paranoid about the types of the linked functions. So I would write that part as
auto getItems = static_cast<GetItemsFn>(GetProcAddress(lib, "get_items"));
I can only assume the original code is more complex and has a good reason to have this weird way of doing things, while the example is oversimplified.
Ouch. So it works basically like C's null-terminated strings. A very bad and error-prone idea. I can only assume you don't control the source of the library, but if you do, strongly consider changing that type.
Important question: the returned type is a pointer. Who owns the pointed to data? Is it an owned pointer that you must manually free later, or is it a non-owned pointer? If so, are you allowed to write to the pointed data? And what is its lifetime bound to? Those are important questions if you want to properly wrap that code in Rust.
Of course, if we're talking about a one-off code rather than a proper safe wrapper for some library, it may be fine to use raw pointers a few times, rather than trying to build a proper safe abstraction.
You have two logically independent operations with different preconditions inside a single unsafe block. That's a good way to introduce soundness errors. You should split the block into two parts, which would also allow you to write the code neater, without splitting off the declaration and definition of a variable. Like this:
// SAFETY: provide explanation on the safety of linking a library. Most importantly, this likely
// requires talking about its multithreading safety, and ensuring that you don't violate it (e.g.
// linking a dynamic library concurrently may not be safe, look up your OS and library specifics).
let lib = unsafe { Library::new(dll_path).unwrap() };
// SAFETY: ensure that the type of the linked function is properly specified. Wrong function
// signature is UB.
let get_items_fn = unsafe { lib.get::<fn() -> *const MyItem>(b"get_items").unwrap() };
Indeed, you don't know the length. You can compute it the same way as the original code does: walk the array until you find item.id == 0
. Like this:
/// SAFETY: we must ensure that `items` points to live valid data. The function also does
/// an unchecked walk over the pointed to buffer, which may cause out of bounds errors
/// if the buffer was improperly initialized or the null terminator was overwritten at some point.
unsafe fn get_items_len(items: *const MyItem) -> usize {
for len in in 0.. {
// Do you have any upper bound on the size of buffer? If you do, it would be
// nice to put a guard here, to reduce the possibility of out of bounds access
// if the buffer is malformed.
let item = *items.add(len);
if item.id == 0 { return len; }
}
unreachable!();
}
Now that you have the length, you can pass it to slice::from_raw_parts
and proceed as usual. That said, &[MyItem]
has an important limitation: it can be freely copied and shared between threads, and the pointed to data must be immutable. Are you sure that accesses to *MyItem
are thread-safe and reentrant? You should ensure that other library functions which you're going to call aren't going to do concurrent writes to that buffer, or worse, free or reallocate it from under you.