Skip to content

Commit

Permalink
Implement fallbacks for functions unavailable in older versions of Wi…
Browse files Browse the repository at this point in the history 
…ndows
  • Loading branch information
@MrAlert
MrAlert committed May 4, 2014
1 parent 1f25c8b commit 073d7ff
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 16 deletions.
15 changes: 4 additions & 11 deletions src/liblibc/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ pub use funcs::bsd43::{shutdown};
#[cfg(windows)] pub use funcs::extra::kernel32::{FlushFileBuffers, SetEndOfFile, CreateFileW};
#[cfg(windows)] pub use funcs::extra::kernel32::{CreateDirectoryW, FindFirstFileW};
#[cfg(windows)] pub use funcs::extra::kernel32::{FindNextFileW, FindClose, DeleteFileW};
#[cfg(windows)] pub use funcs::extra::kernel32::{GetFinalPathNameByHandleW, CreateSymbolicLinkW};
#[cfg(windows)] pub use funcs::extra::kernel32::{CreateHardLinkW, CreateEventW};
#[cfg(windows)] pub use funcs::extra::kernel32::{FlushFileBuffers, CreateNamedPipeW};
#[cfg(windows)] pub use funcs::extra::kernel32::{SetNamedPipeHandleState, WaitNamedPipeW};
Expand Down Expand Up @@ -1733,6 +1732,7 @@ pub mod consts {
pub static ERROR_INVALID_HANDLE : c_int = 6;
pub static ERROR_BROKEN_PIPE: c_int = 109;
pub static ERROR_DISK_FULL : c_int = 112;
pub static ERROR_CALL_NOT_IMPLEMENTED : c_int = 120;
pub static ERROR_INSUFFICIENT_BUFFER : c_int = 122;
pub static ERROR_INVALID_NAME : c_int = 123;
pub static ERROR_ALREADY_EXISTS : c_int = 183;
Expand Down Expand Up @@ -4185,9 +4185,9 @@ pub mod funcs {
LPSTARTUPINFO,
LPPROCESS_INFORMATION,
LPMEMORY_BASIC_INFORMATION,
LPSYSTEM_INFO, BOOLEAN,
HANDLE, LPHANDLE, LARGE_INTEGER,
PLARGE_INTEGER, LPFILETIME};
LPSYSTEM_INFO, HANDLE, LPHANDLE,
LARGE_INTEGER, PLARGE_INTEGER,
LPFILETIME};

extern "system" {
pub fn GetEnvironmentVariableW(n: LPCWSTR,
Expand Down Expand Up @@ -4297,9 +4297,6 @@ pub mod funcs {
pub fn MoveFileExW(lpExistingFileName: LPCWSTR,
lpNewFileName: LPCWSTR,
dwFlags: DWORD) -> BOOL;
pub fn CreateSymbolicLinkW(lpSymlinkFileName: LPCWSTR,
lpTargetFileName: LPCWSTR,
dwFlags: DWORD) -> BOOLEAN;
pub fn CreateHardLinkW(lpSymlinkFileName: LPCWSTR,
lpTargetFileName: LPCWSTR,
lpSecurityAttributes: LPSECURITY_ATTRIBUTES)
Expand All @@ -4312,10 +4309,6 @@ pub mod funcs {
dwCreationDisposition: DWORD,
dwFlagsAndAttributes: DWORD,
hTemplateFile: HANDLE) -> HANDLE;
pub fn GetFinalPathNameByHandleW(hFile: HANDLE,
lpszFilePath: LPCWSTR,
cchFilePath: DWORD,
dwFlags: DWORD) -> DWORD;
pub fn ReadFile(hFile: HANDLE,
lpBuffer: LPVOID,
nNumberOfBytesToRead: DWORD,
Expand Down
12 changes: 7 additions & 5 deletions src/libnative/io/file_win32.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ pub fn chown(_p: &CString, _uid: int, _gid: int) -> IoResult<()> {

pub fn readlink(p: &CString) -> IoResult<Path> {
// FIXME: I have a feeling that this reads intermediate symlinks as well.
use std::os::win32::compat::kernel32::GetFinalPathNameByHandleW;
let handle = unsafe {
as_utf16_p(p.as_str().unwrap(), |p| {
libc::CreateFileW(p,
Expand All @@ -425,10 +426,10 @@ pub fn readlink(p: &CString) -> IoResult<Path> {
// Specify (sz - 1) because the documentation states that it's the size
// without the null pointer
let ret = fill_utf16_buf_and_decode(|buf, sz| unsafe {
libc::GetFinalPathNameByHandleW(handle,
buf as *u16,
sz - 1,
libc::VOLUME_NAME_DOS)
GetFinalPathNameByHandleW(handle,
buf as *u16,
sz - 1,
libc::VOLUME_NAME_DOS)
});
let ret = match ret {
Some(ref s) if s.starts_with(r"\\?\") => Ok(Path::new(s.slice_from(4))),
Expand All @@ -440,9 +441,10 @@ pub fn readlink(p: &CString) -> IoResult<Path> {
}

pub fn symlink(src: &CString, dst: &CString) -> IoResult<()> {
use std::os::win32::compat::kernel32::CreateSymbolicLinkW;
super::mkerr_winbool(as_utf16_p(src.as_str().unwrap(), |src| {
as_utf16_p(dst.as_str().unwrap(), |dst| {
unsafe { libc::CreateSymbolicLinkW(dst, src, 0) }
unsafe { CreateSymbolicLinkW(dst, src, 0) }
}) as libc::BOOL
}))
}
Expand Down
78 changes: 78 additions & 0 deletions src/libstd/os.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,84 @@ pub mod win32 {
t.push(0u16);
f(t.as_ptr())
}

pub mod compat {

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton May 4, 2014

I think this module may be better suited in libnative itself, I don't want to encourage too much use of these functions outside of libnative.

This comment has been minimized.

Sign in to view

Copy link
@MrAlert

MrAlert May 7, 2014

Author Owner

Although CreateSymbolicLinkW and GetFinalPathNameByHandleW are used by libnative, I was thinking that this compatibility mechanism would be used for other functions as well. Perhaps even for more than just XP compatibility. (Maybe allow the use of some Windows 8 functions or something?) At least native condition variables, as mentioned in rust-lang#12842, would be outside of libnative as far as I can tell.

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton May 7, 2014

Let's put this in libnative for now, we can move it out if we need to.
use kinds::Copy;
use option::Option;
use c_str::ToCStr;
use intrinsics::{atomic_store_relaxed, transmute};
use libc::types::os::arch::extra::{LPCWSTR, HMODULE, LPCSTR, LPVOID};
use os::win32::as_utf16_p;

#[link_name="kernel32"]

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton May 4, 2014

You can remove this line, it's a relic from an era long since past, and we keep forget to remove all the old ones!
extern "system" {
fn GetModuleHandleW(lpModuleName: LPCWSTR) -> HMODULE;
fn GetProcAddress(hModule: HMODULE, lpProcName: LPCSTR) -> LPVOID;
}

unsafe fn store_func<T: Copy>(ptr: *mut T, module: &str, symbol: &str, fallback: T) {
as_utf16_p(module, |module| {
symbol.with_c_str(|symbol| {
let handle = GetModuleHandleW(module);
let func: Option<T> = transmute(GetProcAddress(handle, symbol));
atomic_store_relaxed(ptr, func.unwrap_or(fallback))
})
})
}

macro_rules! compat_fn(
($module:ident::$symbol:ident($($argname:ident: $argtype:ty),*)
-> $rettype:ty $fallback:block) => (
#[inline(always)]
pub unsafe fn $symbol($($argname: $argtype),*) -> $rettype {
static mut ptr: extern "system" fn($($argname: $argtype),*) -> $rettype = thunk;

extern "system" fn thunk($($argname: $argtype),*) -> $rettype {
unsafe {
::os::win32::compat::store_func(&mut ptr,
stringify!($module),
stringify!($symbol),
fallback);
::intrinsics::atomic_load_relaxed(&ptr)($($argname),*)

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton May 4, 2014

I'm a little worried about these relaxed loads and stores. I suppose the worst that can happen is that the initialization is run more than once. Can you add comments for why you're using a relaxed ordering and why it is acceptable? It also seems like the cost of the syscall far outweighs the strong ordering, but I don't think it matters too much in this use case.

This comment has been minimized.

Sign in to view

Copy link
@Aatch

Aatch May 7, 2014

The only reason is needs to be atomic is just so the pointer is written atomically. There is still the "race" of somebody calling into the thunk from one thread while another is getting the actual symbol. Without locks and such, it's not possible to prevent multiple writes to the pointer.
}
}

extern "system" fn fallback($($argname: $argtype),*) -> $rettype $fallback

::intrinsics::atomic_load_relaxed(&ptr)($($argname),*)

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton May 4, 2014

Can this use the AtomicPtr type directly?

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton May 4, 2014

(or atomic uint)

This comment has been minimized.

Sign in to view

Copy link
@MrAlert

MrAlert May 7, 2014

Author Owner

As far as I can tell, AtomicUint can't be statically initialized with any value other than 0. The way this is implemented, the function pointer is statically initialized to point to the loading thunk, so that further calls can be directly to the function itself, rather than doing any additional branching.

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton May 7, 2014

These are implementing syscalls, which aren't exactly the speediest things in the world, so an initial value of 0 doesn't seem like it would be that bad (a branch on every call)
}
);

($module:ident::$symbol:ident($($argname:ident: $argtype:ty),*) $fallback:block) => (
compat_fn!($module::$symbol($($argname: $argtype),*) -> () $fallback)
)
)

pub mod kernel32 {
use libc::types::os::arch::extra::{DWORD, LPCWSTR, BOOLEAN, HANDLE};
use libc::consts::os::extra::ERROR_CALL_NOT_IMPLEMENTED;

#[link_name="kernel32"]
extern "system" {
fn SetLastError(dwErrCode: DWORD);
}

compat_fn!(kernel32::CreateSymbolicLinkW(_lpSymlinkFileName: LPCWSTR,
_lpTargetFileName: LPCWSTR,
_dwFlags: DWORD) -> BOOLEAN {
unsafe { SetLastError(ERROR_CALL_NOT_IMPLEMENTED as DWORD); }
0
})

compat_fn!(kernel32::GetFinalPathNameByHandleW(_hFile: HANDLE,
_lpszFilePath: LPCWSTR,
_cchFilePath: DWORD,
_dwFlags: DWORD) -> DWORD {
unsafe { SetLastError(ERROR_CALL_NOT_IMPLEMENTED as DWORD); }
0
})

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton May 4, 2014

It would be nice to have documentation as to why these functions need a compatibility layer.
}
}
}

/*
Expand Down

0 comments on commit 073d7ff

Please sign in to comment.