search

danburkert / lmdb-rs

Safe Rust bindings for LMDB
Apache License 2.0
172 stars 87 forks source link

Consider accepting the UNC paths for env on Windows #33

Closed ncloudioj closed 6 years ago

ncloudioj commented 6 years ago

EnvironmentBuilder::open() panics on Windows (latest stable-x86_64-pc-windows-msvc) while calling it with a canonicalized path generated by std::path::Path::canonicalize(). The error_no(123) suggests that the given path is invalid, therefore it couldn't open the environment at that path.

Looks like the canonicalized paths on Windows are UNC paths (e.g. \\?\C:\\foo.bar), if the underlying mmap system calls can't play well with them, shall we consider converting those UNC paths to regular ones here?

danburkert commented 6 years ago

What’s the backtrack of the panic? I’m guessing it’s the as_bytes call which does look a little sketchy, there’s probably a better way to do that.

ncloudioj commented 6 years ago

Turns out, given canonicalized paths on Windows, ffi::mdb_env_open(env, path.as_ptr(), self.flags.bits(), mode) failed with a error code 123, which suggests that the given path (attached below) was invalid.

\\\\?\\C:\\Users\\Administrator\\AppData\\Local\\Temp\\test.E6WJJzsWy3x8

We workaround this by applying this path cleaning to those Windows UNC paths.

Not sure if that's a legit fix for this crate, perhaps we can add a note here for those users who deal with Windows UNC paths.

danburkert commented 6 years ago

I'm not following, is there a panic? If there's a panic could you include the backtrace?

ncloudioj commented 6 years ago

Oh, the panic was caused by the following expect() in my test, not from this crate.

My apologies for the confusion, I've rephrased the title of this issue.

danburkert commented 6 years ago

Ah ok, thanks. I agree then that a doc note about UNC paths will suffice. Since this seems like a widespread issue in Rust I don't really want to solve it specially for this crate.