You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@teaclave.apache.org by GitBox <gi...@apache.org> on 2020/07/18 18:35:11 UTC
[GitHub] [incubator-teaclave-sgx-sdk] dingelish commented on a change in pull request #258: Change sgx_libc Interface for Rust Crates.
dingelish commented on a change in pull request #258:
URL: https://github.com/apache/incubator-teaclave-sgx-sdk/pull/258#discussion_r456812722
##########
File path: sgx_urts/src/net.rs
##########
@@ -15,34 +15,72 @@
// specific language governing permissions and limitations
// under the License..
+use libc::{self, addrinfo, c_char, c_int, size_t};
+use std::ffi::CStr;
use std::io::Error;
-use libc::{self, c_int, c_char, addrinfo};
+use std::slice;
+
+use core::ptr;
#[no_mangle]
pub extern "C" fn u_getaddrinfo_ocall(
error: *mut c_int,
node: *const c_char,
service: *const c_char,
- hints: *const addrinfo,
- res: *mut *mut addrinfo,
+ hints: *mut addrinfo,
+ entry_size: size_t,
+ buf: *mut u8,
+ bufsz: size_t,
+ out_cnt: *mut size_t,
) -> c_int {
let mut errno = 0;
- let ret = unsafe { libc::getaddrinfo(node, service, hints, res) };
+ let mut res: *mut addrinfo = ptr::null_mut();
+
+ let ret = unsafe { libc::getaddrinfo(node, service, hints, &mut res) };
+
if ret == libc::EAI_SYSTEM {
errno = Error::last_os_error().raw_os_error().unwrap_or(0);
}
if !error.is_null() {
- unsafe { *error = errno; }
+ unsafe {
+ *error = errno;
+ }
}
- ret
-}
-#[no_mangle]
-pub extern "C" fn u_freeaddrinfo_ocall(res: *mut addrinfo ) {
- unsafe { libc::freeaddrinfo(res) }
+ if ret != 0 {
+ return ret;
+ }
+
+ unsafe {
+ let mut i = 0;
+ let mut cur_ptr = res;
+ while cur_ptr != ptr::null_mut() && (i + entry_size) < bufsz {
+ let cur: &addrinfo = &*cur_ptr;
+ let len = cur.ai_addrlen as usize;
+ if len > entry_size {
+ return 1;
+ }
+ std::ptr::copy_nonoverlapping(cur.ai_addr as *mut u8, buf.add(i), len);
+ i += entry_size;
+ cur_ptr = cur.ai_next;
+ }
+ *out_cnt = i / entry_size;
+ }
+
+ unsafe { libc::freeaddrinfo(res) };
+ return 0;
}
#[no_mangle]
-pub extern "C" fn u_gai_strerror_ocall(errcode: c_int) -> *const c_char {
- unsafe { libc::gai_strerror(errcode) }
+pub extern "C" fn u_gai_strerror_ocall(errcode: c_int, buf: *mut c_char, bufsz: size_t) -> c_int {
Review comment:
`gai-strerr-strs` seems pretty much stable. can we implement them here in native Rust?
https://github.com/bminor/glibc/commits/2094e1a9757c1aa4307e65ae39ad20e9134f92e8/sysdeps/posix/gai_strerror-strs.h
##########
File path: sgx_urts/src/file.rs
##########
@@ -402,31 +414,35 @@ pub extern "C" fn u_symlink_ocall(
pub extern "C" fn u_realpath_ocall(
error: *mut c_int,
pathname: *const c_char,
-) -> *mut c_char {
+ resolved_buf: *mut c_char,
+ _bufsz: size_t,
+) -> c_int {
let mut errno = 0;
- let ret = unsafe { libc::realpath(pathname, ptr::null_mut()) };
+ let mut result = 0;
+ let ret = unsafe { libc::realpath(pathname, resolved_buf) };
if ret.is_null() {
errno = Error::last_os_error().raw_os_error().unwrap_or(0);
+ result = -1;
Review comment:
`libc::realpath` return `NULL` on error. should we do this as well?
##########
File path: sgx_urts/src/net.rs
##########
@@ -15,34 +15,72 @@
// specific language governing permissions and limitations
// under the License..
+use libc::{self, addrinfo, c_char, c_int, size_t};
+use std::ffi::CStr;
use std::io::Error;
-use libc::{self, c_int, c_char, addrinfo};
+use std::slice;
+
+use core::ptr;
#[no_mangle]
pub extern "C" fn u_getaddrinfo_ocall(
error: *mut c_int,
node: *const c_char,
service: *const c_char,
- hints: *const addrinfo,
- res: *mut *mut addrinfo,
+ hints: *mut addrinfo,
+ entry_size: size_t,
+ buf: *mut u8,
+ bufsz: size_t,
+ out_cnt: *mut size_t,
) -> c_int {
let mut errno = 0;
- let ret = unsafe { libc::getaddrinfo(node, service, hints, res) };
+ let mut res: *mut addrinfo = ptr::null_mut();
+
+ let ret = unsafe { libc::getaddrinfo(node, service, hints, &mut res) };
+
if ret == libc::EAI_SYSTEM {
errno = Error::last_os_error().raw_os_error().unwrap_or(0);
}
if !error.is_null() {
- unsafe { *error = errno; }
+ unsafe {
+ *error = errno;
+ }
}
- ret
-}
-#[no_mangle]
-pub extern "C" fn u_freeaddrinfo_ocall(res: *mut addrinfo ) {
- unsafe { libc::freeaddrinfo(res) }
+ if ret != 0 {
+ return ret;
+ }
+
+ unsafe {
+ let mut i = 0;
+ let mut cur_ptr = res;
+ while cur_ptr != ptr::null_mut() && (i + entry_size) < bufsz {
+ let cur: &addrinfo = &*cur_ptr;
+ let len = cur.ai_addrlen as usize;
+ if len > entry_size {
+ return 1;
+ }
+ std::ptr::copy_nonoverlapping(cur.ai_addr as *mut u8, buf.add(i), len);
Review comment:
cur.ai_addr as * const u8?
##########
File path: sgx_urts/src/file.rs
##########
@@ -461,9 +481,17 @@ pub extern "C" fn u_opendir_ocall(error: *mut c_int, pathname: *const c_char) ->
pub extern "C" fn u_readdir64_r_ocall(
dirp: *mut DIR,
entry: *mut dirent64,
- result: *mut *mut dirent64,
+ hint: *mut c_int,
) -> c_int {
- unsafe { libc::readdir64_r(dirp, entry, result) }
+ let mut result_ptr = ptr::null_mut();
+ let result = unsafe { libc::readdir64_r(dirp, entry, &mut result_ptr) };
+ assert!(!hint.is_null());
+ if !result_ptr.is_null() {
+ unsafe {
Review comment:
recommend check `hint` inside this block and return `EOVERFLOW`(One of the values in the structure to be returned cannot be represented correctly.) if `NULL` so we can remove this `assert`.
##########
File path: sgx_urts/src/net.rs
##########
@@ -15,34 +15,72 @@
// specific language governing permissions and limitations
// under the License..
+use libc::{self, addrinfo, c_char, c_int, size_t};
+use std::ffi::CStr;
use std::io::Error;
-use libc::{self, c_int, c_char, addrinfo};
+use std::slice;
+
+use core::ptr;
#[no_mangle]
pub extern "C" fn u_getaddrinfo_ocall(
error: *mut c_int,
node: *const c_char,
service: *const c_char,
- hints: *const addrinfo,
- res: *mut *mut addrinfo,
+ hints: *mut addrinfo,
+ entry_size: size_t,
+ buf: *mut u8,
+ bufsz: size_t,
+ out_cnt: *mut size_t,
) -> c_int {
let mut errno = 0;
- let ret = unsafe { libc::getaddrinfo(node, service, hints, res) };
+ let mut res: *mut addrinfo = ptr::null_mut();
+
+ let ret = unsafe { libc::getaddrinfo(node, service, hints, &mut res) };
+
if ret == libc::EAI_SYSTEM {
errno = Error::last_os_error().raw_os_error().unwrap_or(0);
}
if !error.is_null() {
- unsafe { *error = errno; }
+ unsafe {
+ *error = errno;
+ }
}
- ret
-}
-#[no_mangle]
-pub extern "C" fn u_freeaddrinfo_ocall(res: *mut addrinfo ) {
- unsafe { libc::freeaddrinfo(res) }
+ if ret != 0 {
+ return ret;
+ }
+
+ unsafe {
+ let mut i = 0;
+ let mut cur_ptr = res;
+ while cur_ptr != ptr::null_mut() && (i + entry_size) < bufsz {
+ let cur: &addrinfo = &*cur_ptr;
+ let len = cur.ai_addrlen as usize;
+ if len > entry_size {
+ return 1;
Review comment:
1 = `EAI_ADDRFAMILY`. recommend return `EAI_SYSTEM`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@teaclave.apache.org
For additional commands, e-mail: notifications-help@teaclave.apache.org