Closed rami3l closed 1 week ago
How about this: #3120
It allows you to gracefully handle registry values even when their byte values are not necessarily valid or corresponding to their purported type. You can thus query a "string" value as bytes and interpret it however you wish.
Let me know what you think.
How about this: #3120
It allows you to gracefully handle registry values even when their byte values are not necessarily valid or corresponding to their purported type. You can thus query a "string" value as bytes and interpret it however you wish.
Let me know what you think.
@kennykerr Thanks for the quick response! After a quick look it seems that this only solves half the problem though: since we're modifying the PATH
, we have to find a way to put the value back after appending to it...
I still personally prefer the OsStr
approach, as it's quite clear that:
String
might very well fail due to encoding problems. That error is better handled on our side rather than by the library.How about if we added get_os_string
and set_os_string
? That way folks that need no_std
support or just prefer str
can still use windows-registry
and folks that prefer/need OsStr
can opt for that as well.
How about if we added
get_os_string
andset_os_string
? That way folks that needno_std
support or just preferstr
can still usewindows-registry
and folks that prefer/needOsStr
can opt for that as well.
@kennykerr Sounds good!
OsString
support.
Hi, and thanks for making this project!
Background
Coming from https://github.com/rust-lang/rustup/pull/3896, we (the Rustup team) are currently evaluating the possibility of replacing
winreg
withwindows-registry
in Rustup. This is mainly because we use the registry APIs to inspect and change thePATH
variable, which is a necessary step in Rustup's installation process.However, the following function signatures look a bit concerning:
https://github.com/microsoft/windows-rs/blob/2ec4e06d366c46ba9634316959f680ea3dc42a5a/crates/libs/registry/src/key.rs#L245 https://github.com/microsoft/windows-rs/blob/2ec4e06d366c46ba9634316959f680ea3dc42a5a/crates/libs/registry/src/key.rs#L95
... which is because being valid UTF8 is a core invariant of the
str
type. OTOH, it seems to me that there's no guarantee that we should always get/set valid UTF16LE from/to the registry, and we'd like to make sure that Rustup is still working correctly even if there are non-Unicode bytes in the registry value in question (which could be added by a third-party app?).For example, we now have the following smoke test that inserts into the registry a sequence of
u16
that would be considered malformed UTF16LE, and it's expected to work:_https://github.com/rust-lang/rustup/blob/4c3ff9ce638e8402fbc2e8755d9e8a2cd148f6be/tests/suite/cli_paths.rs#L427-L440_
Suggestion
For the exact reason explained above, I believe
OsStr
/OsString
is a better alternative tostr
/String
in APIs like this.Quoting https://doc.rust-lang.org/std/ffi/struct.OsString.html:
Once that change is made, we would be able to use
windows::ffi::OsStringExt::from_wide
to construct anOsString
from any&[u16]
.I'm still not very familiar with Windows development myself, so please feel free to correct me if I'm wrong about anything. Many thanks in advance!