Skip to content

Commit

Permalink
Change InfoType to_str handling for #2.
Browse files Browse the repository at this point in the history 
Remove `Str` enum value, use `VecUchar` instead.
Add `to_str_unchecked` method to convert a  `VecUchar` into a `CString` including all `nul` characters.
  • Loading branch information
@kenba
kenba committed Mar 18, 2021
1 parent 9a7eb46 commit fdbf953
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 25 deletions.
2 changes: 1 addition & 1 deletion src/device.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ pub fn get_device_info(device: cl_device_id, param_name: DeviceInfo) -> Result<I
=> {
api_info_vector!(get_string, u8, clGetDeviceInfo);
let size = get_size(device, param_id)?;
Ok(InfoType::Str(get_string(device, param_id, size)?))
Ok(InfoType::VecUchar(get_string(device, param_id, size)?))
}

DeviceInfo::CL_DEVICE_VENDOR_ID
Expand Down
28 changes: 18 additions & 10 deletions src/info_type.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use std::ffi::{CString, NulError};
/// The functions will panic if they are called for the incorrect type.
#[derive(Debug)]
pub enum InfoType {
Str(Vec<u8>),
Int(cl_int),
Uint(cl_uint),
Ulong(cl_ulong),
Expand All @@ -38,16 +37,25 @@ pub enum InfoType {

impl InfoType {
pub fn to_str(self) -> Result<CString, NulError> {
match self {
InfoType::Str(mut a) => {
// remove all trailing nulls if any
while let Some(0) = a.last() {
a.pop();
}
CString::new(a)
}
_ => panic!("not a String"),
let mut a = self.to_vec_uchar();

// remove all trailing nulls if any

This comment has been minimized.

Sign in to view

Copy link
@vmx

vmx Mar 19, 2021

Contributor

With this implementation I think the approach would be a Python-like one. When you call it, it is best effort and if it fails you deal with it. So you want to make sure you always get a valid CString you would do something like:

match build_log.to_str() {
    Ok(log) => log,
    Err(nul_error) => {
        CString::new(
            std::str::from_utf8(&nul_error.into_vec())
                .unwrap()
                .replace('\0', "")
        )
   }
}

I personally would expect cl3 do that for me. I see cl3 as Rust abstraction on top of the FFI, which also fixes some low level quirks (like having nuls in strings) to make it usable in Rust. An abstraction that just doesn't pass on the quirks from the FFI.
while let Some(0) = a.last() {
a.pop();
}

CString::new(a)
}

pub unsafe fn to_str_unchecked(self) -> CString {
let mut a = self.to_vec_uchar();

// remove all trailing nulls if any
while let Some(0) = a.last() {
a.pop();
}

CString::from_vec_unchecked(a)
}

pub fn to_int(self) -> cl_int {
Expand Down
36 changes: 26 additions & 10 deletions src/kernel.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ pub fn get_kernel_info(kernel: cl_kernel, param_name: KernelInfo) -> Result<Info
KernelInfo::CL_KERNEL_FUNCTION_NAME | KernelInfo::CL_KERNEL_ATTRIBUTES => {
api_info_vector!(get_string, u8, clGetKernelInfo);
let size = get_size(kernel, param_id)?;
Ok(InfoType::Str(get_string(kernel, param_id, size)?))
Ok(InfoType::VecUchar(get_string(kernel, param_id, size)?))
}

KernelInfo::CL_KERNEL_NUM_ARGS | KernelInfo::CL_KERNEL_REFERENCE_COUNT => {
Expand Down Expand Up @@ -309,7 +309,7 @@ pub fn get_kernel_arg_info(
api2_info_size!(get_device_size, cl_uint, clGetKernelArgInfo);
api2_info_vector!(get_device_string, cl_uint, u8, clGetKernelArgInfo);
let size = get_device_size(kernel, arg_indx, param_id)?;
Ok(InfoType::Str(get_device_string(
Ok(InfoType::VecUchar(get_device_string(
kernel, arg_indx, param_id, size,
)?))
}
Expand Down Expand Up @@ -491,9 +491,9 @@ mod tests {
use super::*;
use crate::context::{create_context, release_context};
use crate::device::{get_device_ids, CL_DEVICE_TYPE_GPU};
use crate::error_codes::error_text;
use crate::platform::get_platform_ids;
use crate::program::{build_program, create_program_with_source, release_program};
use crate::error_codes::error_text;
use std::ffi::CString;

#[test]
Expand Down Expand Up @@ -569,15 +569,21 @@ mod tests {
let value = value.to_uint();
println!("CL_KERNEL_ARG_ADDRESS_QUALIFIER: {:X}", value)
}
Err(e) => println!("OpenCL error, CL_KERNEL_ARG_ADDRESS_QUALIFIER: {}", error_text(e))
Err(e) => println!(
"OpenCL error, CL_KERNEL_ARG_ADDRESS_QUALIFIER: {}",
error_text(e)
),
}

match get_kernel_arg_info(kernel, 0, KernelArgInfo::CL_KERNEL_ARG_ACCESS_QUALIFIER) {
Ok(value) => {
let value = value.to_uint();
println!("CL_KERNEL_ARG_ACCESS_QUALIFIER: {:X}", value)
}
Err(e) => println!("OpenCL error, CL_KERNEL_ARG_ACCESS_QUALIFIER: {}", error_text(e))
Err(e) => println!(
"OpenCL error, CL_KERNEL_ARG_ACCESS_QUALIFIER: {}",
error_text(e)
),
}

match get_kernel_arg_info(kernel, 0, KernelArgInfo::CL_KERNEL_ARG_TYPE_NAME) {
Expand All @@ -586,15 +592,18 @@ mod tests {
println!("CL_KERNEL_ARG_TYPE_NAME: {:?}", value);
assert!(0 < value.to_bytes().len())
}
Err(e) => println!("OpenCL error, CL_KERNEL_ARG_TYPE_NAME: {}", error_text(e))
Err(e) => println!("OpenCL error, CL_KERNEL_ARG_TYPE_NAME: {}", error_text(e)),
}

match get_kernel_arg_info(kernel, 0, KernelArgInfo::CL_KERNEL_ARG_TYPE_QUALIFIER) {
Ok(value) => {
let value = value.to_ulong();
println!("CL_KERNEL_ARG_TYPE_QUALIFIER: {:X}", value)
}
Err(e) => println!("OpenCL error, CL_KERNEL_ARG_TYPE_QUALIFIER: {}", error_text(e))
Err(e) => println!(
"OpenCL error, CL_KERNEL_ARG_TYPE_QUALIFIER: {}",
error_text(e)
),
}

match get_kernel_arg_info(kernel, 0, KernelArgInfo::CL_KERNEL_ARG_NAME) {
Expand All @@ -603,7 +612,7 @@ mod tests {
println!("CL_KERNEL_ARG_NAME: {:?}", value);
assert!(0 < value.to_bytes().len())
}
Err(e) => println!("OpenCL error, CL_KERNEL_ARG_NAME: {}", error_text(e))
Err(e) => println!("OpenCL error, CL_KERNEL_ARG_NAME: {}", error_text(e)),
}

let value = get_kernel_work_group_info(
Expand Down Expand Up @@ -651,12 +660,19 @@ mod tests {
let value = value.to_ulong();
println!("CL_KERNEL_PRIVATE_MEM_SIZE: {}", value);

match get_kernel_work_group_info(kernel, device_id, KernelWorkGroupInfo::CL_KERNEL_GLOBAL_WORK_SIZE) {
match get_kernel_work_group_info(
kernel,
device_id,
KernelWorkGroupInfo::CL_KERNEL_GLOBAL_WORK_SIZE,
) {
Ok(value) => {
let value = value.to_vec_size();
println!("CL_KERNEL_GLOBAL_WORK_SIZE: {}", value.len())
}
Err(e) => println!("OpenCL error, CL_KERNEL_GLOBAL_WORK_SIZE: {}", error_text(e))
Err(e) => println!(
"OpenCL error, CL_KERNEL_GLOBAL_WORK_SIZE: {}",
error_text(e)
),
}

release_kernel(kernel).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion src/platform.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ pub fn get_platform_info(
| PlatformInfo::CL_PLATFORM_EXTENSIONS => {
api_info_vector!(get_string, u8, clGetPlatformInfo);
let size = get_size(platform, param_id)?;
Ok(InfoType::Str(get_string(platform, param_id, size)?))
Ok(InfoType::VecUchar(get_string(platform, param_id, size)?))
}
// CL_VERSION_3_0
PlatformInfo::CL_PLATFORM_NUMERIC_VERSION => {
Expand Down
8 changes: 5 additions & 3 deletions src/program.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ pub fn get_program_info(
ProgramInfo::CL_PROGRAM_SOURCE | ProgramInfo::CL_PROGRAM_KERNEL_NAMES | ProgramInfo::CL_PROGRAM_IL => {
api_info_vector!(get_string, u8, clGetProgramInfo);
let size = get_size(program, param_id)?;
Ok(InfoType::Str(get_string(program, param_id, size)?))
Ok(InfoType::VecUchar(get_string(program, param_id, size)?))
}

ProgramInfo::CL_PROGRAM_BINARY_SIZES => {
Expand Down Expand Up @@ -586,7 +586,7 @@ pub fn get_program_build_info(
api2_info_size!(get_device_size, cl_device_id, clGetProgramBuildInfo);
api2_info_vector!(get_device_string, cl_device_id, u8, clGetProgramBuildInfo);
let size = get_device_size(program, device, param_id)?;
Ok(InfoType::Str(get_device_string(program, device, param_id, size)?))
Ok(InfoType::VecUchar(get_device_string(program, device, param_id, size)?))
}

ProgramBuildInfo::CL_PROGRAM_BINARY_TYPE => {
Expand Down Expand Up @@ -690,9 +690,11 @@ mod tests {
let value = value.to_str().unwrap();
println!("CL_PROGRAM_BUILD_OPTIONS: {:?}", value);

unsafe {
let value = get_program_build_info(program, device_id, ProgramBuildInfo::CL_PROGRAM_BUILD_LOG).unwrap();
let value = value.to_str().unwrap();
let value = value.to_str_unchecked();
println!("CL_PROGRAM_BUILD_LOG: {:?}", value);
}

let value = get_program_build_info(program, device_id, ProgramBuildInfo::CL_PROGRAM_BINARY_TYPE).unwrap();
let value = value.to_uint();
Expand Down

0 comments on commit fdbf953

Please sign in to comment.