Skip to content

Commit

Permalink
Change dynamic_library::open_external to take ToCStr
Browse files Browse the repository at this point in the history
`std::unstable::dynamic_library::open_external` currently takes a
`Path`, but because `Paths` produce normalized strings, this can
change the semantics of lookups in a given environment. This patch
generalizes the function to take a `ToCStr`-bounded type, which
includes both `Path`s and `str`s.

Closes #11650.
  • Loading branch information
aturon committed May 15, 2014
1 parent 046062d commit e71202a
Showing 1 changed file with 23 additions and 8 deletions.
31 changes: 23 additions & 8 deletions src/libstd/unstable/dynamic_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,22 @@ impl Drop for DynamicLibrary {
}

impl DynamicLibrary {
// FIXME (#12938): Until DST lands, we cannot decompose &str into
// & and str, so we cannot usefully take ToCStr arguments by
// reference (without forcing an additional & around &str). So we
// are instead temporarily adding an instance for &Path, so that
// we can take ToCStr as owned. When DST lands, the &Path instance
// should be removed, and arguments bound by ToCStr should be
// passed by reference. (Here: in the `open` method.)

/// Lazily open a dynamic library. When passed None it gives a
/// handle to the calling process
pub fn open(filename: Option<&path::Path>) -> Result<DynamicLibrary, ~str> {
pub fn open<T: ToCStr>(filename: Option<T>)
-> Result<DynamicLibrary, ~str> {
unsafe {
let mut filename = filename;
let maybe_library = dl::check_for_errors_in(|| {
match filename {
match filename.take() {
Some(name) => dl::open_external(name),
None => dl::open_internal()
}
Expand Down Expand Up @@ -114,7 +124,8 @@ mod test {
fn test_loading_cosine() {
// The math library does not need to be loaded since it is already
// statically linked in
let libm = match DynamicLibrary::open(None) {
let none: Option<Path> = None; // appease the typechecker
let libm = match DynamicLibrary::open(none) {
Err(error) => fail!("Could not load self as module: {}", error),
Ok(libm) => libm
};
Expand Down Expand Up @@ -142,7 +153,7 @@ mod test {
fn test_errors_do_not_crash() {
// Open /dev/null as a library to get an error, and make sure
// that only causes an error, and not a crash.
let path = GenericPath::new("/dev/null");
let path = Path::new("/dev/null");
match DynamicLibrary::open(Some(&path)) {
Err(_) => {}
Ok(_) => fail!("Successfully opened the empty library.")
Expand All @@ -157,12 +168,11 @@ mod test {
pub mod dl {
use c_str::ToCStr;
use libc;
use path;
use ptr;
use str;
use result::*;

pub unsafe fn open_external(filename: &path::Path) -> *u8 {
pub unsafe fn open_external<T: ToCStr>(filename: T) -> *u8 {
filename.with_c_str(|raw_name| {
dlopen(raw_name, Lazy as libc::c_int) as *u8
})
Expand Down Expand Up @@ -223,9 +233,14 @@ pub mod dl {
use os;
use ptr;
use result::{Ok, Err, Result};
use str;
use c_str::ToCStr;

pub unsafe fn open_external(filename: &path::Path) -> *u8 {
os::win32::as_utf16_p(filename.as_str().unwrap(), |raw_name| {
pub unsafe fn open_external<T: ToCStr>(filename: T) -> *u8 {
// Windows expects Unicode data
let filename_cstr = filename.to_c_str();
let filename_str = str::from_utf8(filename_cstr.as_bytes_no_nul()).unwrap();
os::win32::as_utf16_p(filename_str, |raw_name| {
LoadLibraryW(raw_name as *libc::c_void) as *u8
})
}
Expand Down

5 comments on commit e71202a

@bors
Copy link
Contributor

@bors bors commented on e71202a May 15, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from alexcrichton
at aturon@e71202a

@bors
Copy link
Contributor

@bors bors commented on e71202a May 15, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging aturon/rust/issue-11650 = e71202a into auto

@bors
Copy link
Contributor

@bors bors commented on e71202a May 15, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aturon/rust/issue-11650 = e71202a merged ok, testing candidate = fbd8f4a

@bors
Copy link
Contributor

@bors bors commented on e71202a May 15, 2014

@bors
Copy link
Contributor

@bors bors commented on e71202a May 15, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = fbd8f4a

Please sign in to comment.