Skip to content

Commit

Permalink
rustuv: Handle short writes in uv_fs_write
Browse files Browse the repository at this point in the history
The libuv fs wrappers are very thin wrappers around the syscalls they correspond
to, and a notable worrisome case is the write syscall. This syscall is not
guaranteed to write the entire buffer provided, so we may have to continue
calling uv_fs_write if a short write occurs.

Closes #13130
  • Loading branch information
alexcrichton committed Mar 25, 2014
1 parent 1f5571a commit 5fddb42
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 11 deletions.
37 changes: 29 additions & 8 deletions src/librustuv/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::c_str::CString;
use std::c_str;
use std::cast::transmute;
use std::cast;
use std::libc::{c_int, c_char, c_void, size_t};
use std::libc::{c_int, c_char, c_void, size_t, ssize_t};
use std::libc;
use std::rt::task::BlockedTask;
use std::io::{FileStat, IoError};
Expand Down Expand Up @@ -74,11 +74,32 @@ impl FsRequest {
pub fn write(loop_: &Loop, fd: c_int, buf: &[u8], offset: i64)
-> Result<(), UvError>
{
execute_nop(|req, cb| unsafe {
uvll::uv_fs_write(loop_.handle, req,
fd, buf.as_ptr() as *c_void,
buf.len() as size_t, offset, cb)
})
// In libuv, uv_fs_write is basically just shelling out to a write()
// syscall at some point, with very little fluff around it. This means
// that write() could actually be a short write, so we need to be sure
// to call it continuously if we get a short write back. This method is
// expected to write the full data if it returns success.
let mut written = 0;
while written < buf.len() {
let offset = if offset == -1 {
offset
} else {
offset + written as i64
};
match execute(|req, cb| unsafe {
uvll::uv_fs_write(loop_.handle,
req,
fd,
buf.as_ptr().offset(written as int) as *c_void,
(buf.len() - written) as size_t,
offset,
cb)
}).map(|req| req.get_result()) {
Err(e) => return Err(e),
Ok(n) => { written += n as uint; }
}
}
Ok(())
}

pub fn read(loop_: &Loop, fd: c_int, buf: &mut [u8], offset: i64)
Expand Down Expand Up @@ -227,7 +248,7 @@ impl FsRequest {
})
}

pub fn get_result(&self) -> c_int {
pub fn get_result(&self) -> ssize_t {
unsafe { uvll::get_result_from_fs_req(self.req) }
}

Expand Down Expand Up @@ -309,7 +330,7 @@ fn execute(f: |*uvll::uv_fs_t, uvll::uv_fs_cb| -> c_int)
unsafe { uvll::set_data_for_req(req.req, &slot) }
});
match req.get_result() {
n if n < 0 => Err(UvError(n)),
n if n < 0 => Err(UvError(n as i32)),
_ => Ok(req),
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustuv/uvll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ pub unsafe fn set_stdio_container_stream(c: *uv_stdio_container_t,
}

// data access helpers
pub unsafe fn get_result_from_fs_req(req: *uv_fs_t) -> c_int {
pub unsafe fn get_result_from_fs_req(req: *uv_fs_t) -> ssize_t {
rust_uv_get_result_from_fs_req(req)
}
pub unsafe fn get_ptr_from_fs_req(req: *uv_fs_t) -> *libc::c_void {
Expand Down Expand Up @@ -501,7 +501,7 @@ extern {
fn rust_uv_get_udp_handle_from_send_req(req: *uv_udp_send_t) -> *uv_udp_t;

fn rust_uv_populate_uv_stat(req_in: *uv_fs_t, stat_out: *uv_stat_t);
fn rust_uv_get_result_from_fs_req(req: *uv_fs_t) -> c_int;
fn rust_uv_get_result_from_fs_req(req: *uv_fs_t) -> ssize_t;
fn rust_uv_get_ptr_from_fs_req(req: *uv_fs_t) -> *libc::c_void;
fn rust_uv_get_path_from_fs_req(req: *uv_fs_t) -> *c_char;
fn rust_uv_get_loop_from_fs_req(req: *uv_fs_t) -> *uv_loop_t;
Expand Down
2 changes: 1 addition & 1 deletion src/rt/rust_uv.c
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ rust_uv_req_type_max() {
return UV_REQ_TYPE_MAX;
}

int
ssize_t
rust_uv_get_result_from_fs_req(uv_fs_t* req) {
return req->result;
}
Expand Down

5 comments on commit 5fddb42

@bors
Copy link
Contributor

@bors bors commented on 5fddb42 Mar 26, 2014

Choose a reason for hiding this comment

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

saw approval from brson
at alexcrichton@5fddb42

@bors
Copy link
Contributor

@bors bors commented on 5fddb42 Mar 26, 2014

Choose a reason for hiding this comment

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

merging alexcrichton/rust/issue-13130 = 5fddb42 into auto

@bors
Copy link
Contributor

@bors bors commented on 5fddb42 Mar 26, 2014

Choose a reason for hiding this comment

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

alexcrichton/rust/issue-13130 = 5fddb42 merged ok, testing candidate = 82c8cb2

@bors
Copy link
Contributor

@bors bors commented on 5fddb42 Mar 26, 2014

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

@bors bors commented on 5fddb42 Mar 26, 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 = 82c8cb2

Please sign in to comment.