Skip to content

Commit

Permalink
Replace use of flow_context_calloc/destroy with AllocationContainer
Browse files Browse the repository at this point in the history
  • Loading branch information
lilith committed Oct 27, 2020
1 parent 7d309b7 commit f145ea0
Show file tree
Hide file tree
Showing 10 changed files with 106 additions and 44 deletions.
4 changes: 2 additions & 2 deletions bindings/headers/imageflow_default.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@ void *imageflow_context_memory_allocate(struct imageflow_context *context,
//
bool imageflow_context_memory_free(struct imageflow_context *context,
void *pointer,
const char *filename,
int32_t line);
const char *_filename,
int32_t _line);

// Prints the error to stderr and exits the process if an error has been raised on the context.
// If no error is present, the function returns false.
Expand Down
4 changes: 2 additions & 2 deletions bindings/headers/imageflow_lua.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ void *imageflow_context_memory_allocate(struct imageflow_context *context,
//
bool imageflow_context_memory_free(struct imageflow_context *context,
void *pointer,
const char *filename,
int32_t line);
const char *_filename,
int32_t _line);

// Prints the error to stderr and exits the process if an error has been raised on the context.
// If no error is present, the function returns false.
Expand Down
4 changes: 2 additions & 2 deletions bindings/headers/imageflow_pinvoke.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@ void *imageflow_context_memory_allocate(void *context,
//
bool imageflow_context_memory_free(void *context,
void *pointer,
const char *filename,
int32_t line);
const char *_filename,
int32_t _line);

// Prints the error to stderr and exits the process if an error has been raised on the context.
// If no error is present, the function returns false.
Expand Down
4 changes: 2 additions & 2 deletions bindings/headers/imageflow_short.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ void *imageflow_context_memory_allocate(void *context,

bool imageflow_context_memory_free(void *context,
void *pointer,
const char *filename,
int32_t line);
const char *_filename,
int32_t _line);

bool imageflow_context_print_and_exit_if_error(void *context);

Expand Down
42 changes: 20 additions & 22 deletions imageflow_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ extern crate imageflow_core as c;
extern crate libc;
extern crate smallvec;
extern crate backtrace;
use crate::c::ffi;

pub use crate::c::{Context, ErrorCategory};
pub use crate::c::ffi::ImageflowJsonResponse as JsonResponse;
Expand Down Expand Up @@ -592,22 +591,19 @@ pub fn create_abi_json_response(c: &mut Context,
let sizeof_struct = std::mem::size_of::<JsonResponse>();
let alloc_size = sizeof_struct + json_bytes.len();

let pointer = crate::ffi::flow_context_calloc(c.flow_c(),
1,
alloc_size,
ptr::null(),
c.flow_c() as *mut libc::c_void,
ptr::null(),
line!() as i32) as *mut u8;
if pointer.is_null() {
c.outward_error_mut().try_set_error(nerror!(ErrorKind::AllocationFailed, "Failed to allocate JsonResponse ({} bytes)", alloc_size));
return ptr::null();
}
if json_bytes.len().leading_zeros() == 0{
c.outward_error_mut().try_set_error(nerror!(ErrorKind::Category(ErrorCategory::InternalError), "Error in creating JSON structure; length overflow prevented (most significant bit set)."));
return ptr::null();
}

let pointer = match c.mem_calloc(alloc_size, 16, ptr::null(), -1){
Err(e) => {
c.outward_error_mut().try_set_error(e);
return ptr::null();
}
Ok(v) => v
};

let pointer_to_final_buffer =
pointer.offset(sizeof_struct as isize) as *mut u8;
let imageflow_response = &mut (*(pointer as *mut JsonResponse));
Expand Down Expand Up @@ -784,16 +780,20 @@ pub extern "C" fn imageflow_context_memory_allocate(context: *mut Context,
bytes: libc::size_t,
filename: *const libc::c_char,
line: i32) -> *mut libc::c_void {

let c = context_ready!(context);

if bytes.leading_zeros() == 0{
if bytes.leading_zeros() == 0 {
c.outward_error_mut().try_set_error(nerror!(ErrorKind::InvalidArgument, "Argument `bytes` likely came from a negative integer. Imageflow prohibits having the leading bit set on unsigned integers (this reduces the maximum value to 2^31 or 2^63)."));
return ptr::null_mut();
}
unsafe {
ffi::flow_context_calloc(c.flow_c(), 1, bytes, ptr::null(), c.flow_c() as *const libc::c_void, filename, line)
}
let pointer = match unsafe{ c.mem_calloc(bytes, 16, filename, line) }{
Err(e) => {
c.outward_error_mut().try_set_error(e);
return ptr::null_mut();
}
Ok(v) => v
};
pointer as *mut libc::c_void
}

///
Expand All @@ -807,13 +807,11 @@ pub extern "C" fn imageflow_context_memory_allocate(context: *mut Context,
#[no_mangle]
pub extern "C" fn imageflow_context_memory_free(context: *mut Context,
pointer: *mut libc::c_void,
filename: *const libc::c_char,
line: i32) -> bool {
_filename: *const libc::c_char,
_line: i32) -> bool {
let c = context!(context); // We must be able to free in an errored state
if !pointer.is_null() {
unsafe {
ffi::flow_destroy(c.flow_c(), pointer, filename, line)
}
unsafe{ c.mem_free(pointer as *const u8) }
}else {
true
}
Expand Down
31 changes: 31 additions & 0 deletions imageflow_core/src/allocation_container.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
use crate::graphics::aligned_buffer::{AlignedBuffer, AlignedBufferError};

pub struct AllocationContainer{
allocations: Vec<AlignedBuffer<u8>>
}
impl AllocationContainer{

pub fn new() -> AllocationContainer{
AllocationContainer{
allocations: Vec::new()
}
}
/// Allocates the specified number of bytes with the given alignment and returns a pointer
pub fn allocate(&mut self, bytes: usize, alignment: usize) -> Result<*mut u8, AlignedBufferError>{
let mut buffer = AlignedBuffer::new(bytes, alignment)?;
let ptr = buffer.as_slice_mut().as_mut_ptr();
self.allocations.push(buffer);
Ok(ptr)
}
/// Returns true if the pointer was found and freed.
pub fn free(&mut self, pointer: *const u8) -> bool{
match self.allocations.iter()
.position(|a| a.as_slice().as_ptr() == pointer) {
Some(index) => {
self.allocations.remove(index);
true
},
None => false
}
}
}
48 changes: 44 additions & 4 deletions imageflow_core/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::codecs::CodecInstanceContainer;
use crate::codecs::EnabledCodecs;
use crate::ffi::IoDirection;
use crate::graphics::bitmaps::{BitmapWindowMut, BitmapKey, Bitmap, BitmapsContainer};
use crate::allocation_container::AllocationContainer;

/// Something of a god object (which is necessary for a reasonable FFI interface).
pub struct Context {
Expand All @@ -37,7 +38,9 @@ pub struct Context {

pub security: imageflow_types::ExecutionSecurity,

pub bitmaps: RefCell<crate::graphics::bitmaps::BitmapsContainer>
pub bitmaps: RefCell<crate::graphics::bitmaps::BitmapsContainer>,

pub allocations: RefCell<AllocationContainer>
}

//TODO: isn't this supposed to increment with each new context in process?
Expand Down Expand Up @@ -75,7 +78,8 @@ impl Context {
megapixels: 100f32
}),
max_encode_size: None
}
},
allocations: RefCell::new(AllocationContainer::new())
}))
}
}
Expand Down Expand Up @@ -127,15 +131,51 @@ impl Context {

pub fn borrow_bitmaps_mut(&self) -> Result<RefMut<BitmapsContainer>>{
self.bitmaps.try_borrow_mut()
.map_err(|e| nerror!(ErrorKind::FailedBorrow, "{:?}", e))
.map_err(|e| nerror!(ErrorKind::FailedBorrow, "Failed to mutably borrow bitmaps collection: {:?}", e))

}
pub fn borrow_bitmaps(&self) -> Result<Ref<BitmapsContainer>>{
self.bitmaps.try_borrow()
.map_err(|e| nerror!(ErrorKind::FailedBorrow, "{:?}", e))
.map_err(|e| nerror!(ErrorKind::FailedBorrow, "Failed to borrow bitmaps collection: {:?}", e))

}

/// mem_calloc should not panic
pub unsafe fn mem_calloc(&self, bytes: usize, alignment: usize,filename: *const libc::c_char, line: i32) -> Result<*mut u8>{
let mut allocations = self.allocations.try_borrow_mut()
.map_err(|e| {
let filename_str = if filename.is_null(){
"[no filename provided]"
} else{
let c_filename = CStr::from_ptr(filename);
c_filename.to_str().unwrap_or("[non UTF-8 filename]")
};

nerror!(ErrorKind::FailedBorrow, "Failed to mutably borrow allocations collection: {:?}\n{}:{}", e, filename_str, line)
})?;

let result = allocations.allocate(bytes, alignment)
.map_err(|e| {
let filename_str = if filename.is_null(){
"[no filename provided]"
} else{
let c_filename = CStr::from_ptr(filename);
c_filename.to_str().unwrap_or("[non UTF-8 filename]")
};

nerror!(ErrorKind::AllocationFailed, "Failed to allocate {} bytes with alignment {}: {:?}\n{}:{}", bytes, alignment, e, filename_str, line)
})?;
Ok(result)
}

/// mem_calloc should not panic
pub unsafe fn mem_free(&self, ptr: *const u8) -> bool{
self.allocations.try_borrow_mut()
.map(|mut list| list.free(ptr))
.unwrap_or(false)
}



pub fn flow_c(&self) -> *mut ffi::ImageflowContext{
self.c_ctx
Expand Down
10 changes: 0 additions & 10 deletions imageflow_core/src/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -811,16 +811,6 @@ mod mid_term {
function_name: *const libc::c_char)
-> bool;

pub fn flow_context_calloc(context: *mut ImageflowContext,
instance_count: usize,
instance_size: usize,
destructor: *const libc::c_void,
owner: *const libc::c_void,
file: *const libc::c_char,
line: i32)
-> *mut libc::c_void;


pub fn flow_bitmap_bgra_populate_histogram(c: *mut ImageflowContext, input: *mut BitmapBgra, histograms: *mut u64, histogram_size_per_channel: u32, histogram_count: u32, pixels_sampled: *mut u64) -> bool;
pub fn flow_bitmap_bgra_apply_color_matrix(c: *mut ImageflowContext, input: *mut BitmapBgra, row: u32, count: u32, matrix: *const *const f32) -> bool;

Expand Down
1 change: 1 addition & 0 deletions imageflow_core/src/graphics/aligned_buffer.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@

#[derive(Debug, Clone)]
pub enum AlignedBufferError{
SizeOverflow,
InvalidAlignment,
Expand Down
2 changes: 2 additions & 0 deletions imageflow_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ pub mod clients;
pub mod ffi;
pub mod parsing;
pub mod test_helpers;
mod allocation_container;

use std::fmt;
use std::borrow::Cow;
use petgraph::graph::NodeIndex;
Expand Down

0 comments on commit f145ea0

Please sign in to comment.