From e7f07450102471a130e3c1f497c04181f5023020 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Thu, 18 Jul 2019 23:03:57 +0200 Subject: [PATCH] reduce direc usage of CString --- src/dc_e2ee.rs | 17 +++++------ src/dc_mimeparser.rs | 5 ++- src/dc_strencode.rs | 9 +++--- src/dc_tools.rs | 72 ++++++++++++++++++++++---------------------- src/imap.rs | 29 +++++++++--------- src/peerstate.rs | 14 +++++---- tests/stress.rs | 34 ++++++++++++--------- 7 files changed, 91 insertions(+), 89 deletions(-) diff --git a/src/dc_e2ee.rs b/src/dc_e2ee.rs index 11714e2c6..8cfbd98ec 100644 --- a/src/dc_e2ee.rs +++ b/src/dc_e2ee.rs @@ -1,5 +1,5 @@ use std::collections::HashSet; -use std::ffi::{CStr, CString}; +use std::ffi::CStr; use std::str::FromStr; use mmime::clist::*; @@ -294,10 +294,8 @@ pub unsafe fn dc_e2ee_encrypt( sign_key.as_ref(), ) { let ctext_bytes = ctext_v.len(); - let ctext_c = CString::new(ctext_v).unwrap(); - let ctext = strdup(ctext_c.as_ptr()); - - (*helper).cdata_to_free = ctext as *mut libc::c_void; + let ctext = to_cstring(ctext_v); + (*helper).cdata_to_free = ctext as *mut _; /* create MIME-structure that will contain the encrypted text */ let mut encrypted_part: *mut mailmime = new_data_part( @@ -354,13 +352,13 @@ pub unsafe fn dc_e2ee_encrypt( 14181132614457621749 => {} _ => { let aheader = Aheader::new(addr, public_key, prefer_encrypt); - let rendered = CString::new(aheader.to_string()).unwrap(); + let rendered = to_cstring(aheader.to_string()); mailimf_fields_add( imffields_unprotected, mailimf_field_new_custom( strdup(b"Autocrypt\x00" as *const u8 as *const libc::c_char), - strdup(rendered.as_ptr()), + rendered, ), ); } @@ -935,13 +933,12 @@ unsafe fn decrypt_part( add_signatures, ) { let plain_bytes = plain.len(); - let plain_c = CString::new(plain).unwrap(); - let plain_buf = strdup(plain_c.as_ptr()); + let plain_buf = plain.as_ptr() as *const libc::c_char; let mut index: size_t = 0i32 as size_t; let mut decrypted_mime: *mut mailmime = 0 as *mut mailmime; if mailmime_parse( - plain_buf as *const libc::c_char, + plain_buf as *const _, plain_bytes, &mut index, &mut decrypted_mime, diff --git a/src/dc_mimeparser.rs b/src/dc_mimeparser.rs index 6efb993b9..6890510fd 100644 --- a/src/dc_mimeparser.rs +++ b/src/dc_mimeparser.rs @@ -1,5 +1,5 @@ use std::collections::{HashMap, HashSet}; -use std::ffi::{CStr, CString}; +use std::ffi::CStr; use charset::Charset; use mmime::mailimf::*; @@ -1204,8 +1204,7 @@ unsafe fn dc_mimeparser_add_single_part_if_known( current_block = 8795901732489102124; } else { decoded_data_bytes = res.len(); - let res_c = CString::new(res.as_bytes()).unwrap(); - decoded_data = strdup(res_c.as_ptr()); + decoded_data = res.as_ptr() as *const libc::c_char; current_block = 17788412896529399552; } } else { diff --git a/src/dc_strencode.rs b/src/dc_strencode.rs index 51501dd87..0af94debd 100644 --- a/src/dc_strencode.rs +++ b/src/dc_strencode.rs @@ -1,4 +1,4 @@ -use std::ffi::{CStr, CString}; +use std::ffi::CStr; use charset::Charset; use mmime::mailmime_decode::*; @@ -689,10 +689,9 @@ pub unsafe fn dc_decode_ext_header(to_decode: *const libc::c_char) -> *mut libc: std::slice::from_raw_parts(decoded as *const u8, strlen(decoded)); let (res, _, _) = encoding.decode(data); - free(decoded as *mut libc::c_void); - let res_c = CString::new(res.as_bytes()).unwrap(); - - decoded = strdup(res_c.as_ptr()); + free(decoded as *mut _); + let r = std::ffi::CString::new(res.as_bytes()).unwrap(); + decoded = dc_strdup(r.as_ptr()); } } } diff --git a/src/dc_tools.rs b/src/dc_tools.rs index fdb3154e5..fdaca5262 100644 --- a/src/dc_tools.rs +++ b/src/dc_tools.rs @@ -177,14 +177,14 @@ pub unsafe fn dc_trim(buf: *mut libc::c_char) { /* the result must be free()'d */ pub unsafe fn dc_strlower(in_0: *const libc::c_char) -> *mut libc::c_char { - let raw = to_cstring(to_string(in_0).to_lowercase()); - strdup(raw.as_ptr()) + to_cstring(to_string(in_0).to_lowercase()) } pub unsafe fn dc_strlower_in_place(in_0: *mut libc::c_char) { let raw = to_cstring(to_string(in_0).to_lowercase()); - assert_eq!(strlen(in_0), strlen(raw.as_ptr())); - memcpy(in_0 as *mut _, raw.as_ptr() as *const _, strlen(in_0)); + assert_eq!(strlen(in_0), strlen(raw)); + memcpy(in_0 as *mut _, raw as *const _, strlen(in_0)); + free(raw as *mut _); } pub unsafe fn dc_str_contains( @@ -232,7 +232,7 @@ pub unsafe fn dc_binary_to_uc_hex(buf: *const uint8_t, bytes: size_t) -> *mut li let buf = std::slice::from_raw_parts(buf, bytes); let raw = hex::encode_upper(buf); - strdup(to_cstring(raw).as_ptr()) + to_cstring(raw) } /* remove all \r characters from string */ @@ -528,7 +528,7 @@ pub unsafe fn dc_str_from_clist( } } - strdup(to_cstring(res).as_ptr()) + to_cstring(res) } pub unsafe fn dc_str_to_clist( @@ -670,7 +670,7 @@ pub unsafe fn dc_timestamp_from_date(date_time: *mut mailimf_date_time) -> i64 { /* the return value must be free()'d */ pub unsafe fn dc_timestamp_to_str(wanted: i64) -> *mut libc::c_char { let res = dc_timestamp_to_str_safe(wanted); - strdup(to_cstring(res).as_ptr()) + to_cstring(res) } pub fn dc_timestamp_to_str_safe(wanted: i64) -> String { @@ -1255,8 +1255,12 @@ pub unsafe fn dc_write_file( } pub fn dc_write_file_safe(context: &Context, pathNfilename: impl AsRef, buf: &[u8]) -> bool { - let pathNfilename_abs = - unsafe { dc_get_abs_path(context, to_cstring(pathNfilename.as_ref()).as_ptr()) }; + let pathNfilename_abs = unsafe { + let n = to_cstring(pathNfilename.as_ref()); + let res = dc_get_abs_path(context, n); + free(n as *mut _); + res + }; if pathNfilename_abs.is_null() { return false; } @@ -1300,8 +1304,13 @@ pub unsafe fn dc_read_file( } pub fn dc_read_file_safe(context: &Context, pathNfilename: impl AsRef) -> Option> { - let pathNfilename_abs = - unsafe { dc_get_abs_path(context, to_cstring(pathNfilename.as_ref()).as_ptr()) }; + let pathNfilename_abs = unsafe { + let n = to_cstring(pathNfilename.as_ref()); + let p = dc_get_abs_path(context, n); + free(n as *mut _); + p + }; + if pathNfilename_abs.is_null() { return None; } @@ -1538,41 +1547,32 @@ fn os_str_to_c_string_unicode( } } -pub fn to_cstring>(s: S) -> CString { - CString::new(s.as_ref()).unwrap() +/// Needs to free the result after use! +pub unsafe fn to_cstring>(s: S) -> *mut libc::c_char { + let cstr = CString::new(s.as_ref()).expect("invalid string converted"); + dc_strdup(cstr.as_ref().as_ptr()) } pub fn to_string(s: *const libc::c_char) -> String { if s.is_null() { return "".into(); } - match unsafe { CStr::from_ptr(s).to_str() } { - Ok(s) => s.to_string(), - Err(err) => { - eprintln!( - "invalid string: '{:?}', {:?}", - unsafe { CStr::from_ptr(s).to_bytes() }, - err - ); - panic!( - "Non utf8 string: '{:?}' ({:?})", - unsafe { CStr::from_ptr(s).to_bytes() }, - err - ); - } - } + + let cstr = unsafe { CStr::from_ptr(s) }; + + cstr.to_str().map(|s| s.to_string()).unwrap_or_else(|err| { + panic!("Non utf8 string: '{:?}' ({:?})", cstr.to_bytes(), err); + }) } pub fn as_str<'a>(s: *const libc::c_char) -> &'a str { assert!(!s.is_null(), "cannot be used on null pointers"); - match unsafe { CStr::from_ptr(s).to_str() } { - Ok(s) => s, - Err(err) => panic!( - "Non utf8 string: '{:?}' ({:?})", - unsafe { CStr::from_ptr(s).to_bytes() }, - err - ), - } + + let cstr = unsafe { CStr::from_ptr(s) }; + + cstr.to_str().unwrap_or_else(|err| { + panic!("Non utf8 string: '{:?}' ({:?})", cstr.to_bytes(), err); + }) } /// Convert a C `*char` pointer to a [std::path::Path] slice. diff --git a/src/imap.rs b/src/imap.rs index e344350cc..48452bc35 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -1,4 +1,3 @@ -use std::ffi::CString; use std::net; use std::sync::{Arc, Condvar, Mutex, RwLock}; use std::time::{Duration, SystemTime}; @@ -6,9 +5,10 @@ use std::time::{Duration, SystemTime}; use crate::constants::*; use crate::context::Context; use crate::dc_loginparam::*; -use crate::dc_tools::as_str; +use crate::dc_tools::{as_str, to_cstring}; use crate::oauth2::dc_get_oauth2_access_token; use crate::types::*; +use crate::x::free; pub const DC_IMAP_SEEN: usize = 0x0001; pub const DC_REGENERATE: usize = 0x01; @@ -706,11 +706,10 @@ impl Imap { fn get_config_last_seen_uid>(&self, context: &Context, folder: S) -> (u32, u32) { let key = format!("imap.mailbox.{}", folder.as_ref()); let val1 = unsafe { - (self.get_config)( - context, - CString::new(key).unwrap().as_ptr(), - 0 as *const libc::c_char, - ) + let key_c = to_cstring(key); + let val = (self.get_config)(context, key_c, 0 as *const libc::c_char); + free(key_c as *mut _); + val }; if val1.is_null() { return (0, 0); @@ -853,9 +852,11 @@ impl Imap { .message_id .expect("missing message id"); - let message_id_c = CString::new(message_id).unwrap(); if 0 == unsafe { - (self.precheck_imf)(context, message_id_c.as_ptr(), folder.as_ref(), cur_uid) + let message_id_c = to_cstring(message_id); + let res = (self.precheck_imf)(context, message_id_c, folder.as_ref(), cur_uid); + free(message_id_c as *mut _); + res } { // check passed, go fetch the rest if self.fetch_single_msg(context, &folder, cur_uid) == 0 { @@ -924,11 +925,11 @@ impl Imap { let val = format!("{}:{}", uidvalidity, lastseenuid); unsafe { - (self.set_config)( - context, - CString::new(key).unwrap().as_ptr(), - CString::new(val).unwrap().as_ptr(), - ) + let key_c = to_cstring(key); + let val_c = to_cstring(val); + (self.set_config)(context, key_c, val_c); + free(key_c as *mut _); + free(val_c as *mut _); }; } diff --git a/src/peerstate.rs b/src/peerstate.rs index b5518486a..876a300b1 100644 --- a/src/peerstate.rs +++ b/src/peerstate.rs @@ -462,10 +462,12 @@ mod tests { use super::*; use pretty_assertions::assert_eq; - use std::ffi::{CStr, CString}; + use std::ffi::CStr; use tempfile::{tempdir, TempDir}; use crate::context::*; + use crate::dc_tools::to_cstring; + use crate::x::free; #[test] fn test_peerstate_save_to_db() { @@ -520,16 +522,16 @@ mod tests { unsafe fn create_test_context() -> TestContext { let mut ctx = dc_context_new(Some(cb), std::ptr::null_mut(), std::ptr::null_mut()); let dir = tempdir().unwrap(); - let dbfile = CString::new(dir.path().join("db.sqlite").to_str().unwrap()).unwrap(); + let dbfile = to_cstring(dir.path().join("db.sqlite").to_str().unwrap()); assert_eq!( - dc_open(&mut ctx, dbfile.as_ptr(), std::ptr::null()), + dc_open(&mut ctx, dbfile, std::ptr::null()), 1, "Failed to open {}", - CStr::from_ptr(dbfile.as_ptr() as *const libc::c_char) - .to_str() - .unwrap() + CStr::from_ptr(dbfile as *const _).to_str().unwrap() ); + free(dbfile as *mut _); + TestContext { ctx: ctx, dir: dir } } } diff --git a/tests/stress.rs b/tests/stress.rs index b32824ea6..ca8240424 100644 --- a/tests/stress.rs +++ b/tests/stress.rs @@ -1,7 +1,6 @@ //! Stress some functions for testing; if used as a lib, this file is obsolete. use std::collections::HashSet; -use std::ffi::CString; use mmime::mailimf_types::*; use tempfile::{tempdir, TempDir}; @@ -691,7 +690,7 @@ fn test_encryption_decryption() { assert!(ctext.starts_with("-----BEGIN PGP MESSAGE-----")); let ctext_signed_bytes = ctext.len(); - let ctext_signed = CString::new(ctext).unwrap(); + let ctext_signed = to_cstring(ctext); let ctext = dc_pgp_pk_encrypt( original_text as *const libc::c_void, @@ -704,7 +703,7 @@ fn test_encryption_decryption() { assert!(ctext.starts_with("-----BEGIN PGP MESSAGE-----")); let ctext_unsigned_bytes = ctext.len(); - let ctext_unsigned = CString::new(ctext).unwrap(); + let ctext_unsigned = to_cstring(ctext); let mut keyring = Keyring::default(); keyring.add_owned(private_key); @@ -718,7 +717,7 @@ fn test_encryption_decryption() { let mut valid_signatures: HashSet = Default::default(); let plain = dc_pgp_pk_decrypt( - ctext_signed.as_ptr() as *const _, + ctext_signed as *const _, ctext_signed_bytes, &keyring, &public_keyring, @@ -733,7 +732,7 @@ fn test_encryption_decryption() { let empty_keyring = Keyring::default(); let plain = dc_pgp_pk_decrypt( - ctext_signed.as_ptr() as *const _, + ctext_signed as *const _, ctext_signed_bytes, &keyring, &empty_keyring, @@ -746,7 +745,7 @@ fn test_encryption_decryption() { valid_signatures.clear(); let plain = dc_pgp_pk_decrypt( - ctext_signed.as_ptr() as *const _, + ctext_signed as *const _, ctext_signed_bytes, &keyring, &public_keyring2, @@ -761,7 +760,7 @@ fn test_encryption_decryption() { public_keyring2.add_ref(&public_key); let plain = dc_pgp_pk_decrypt( - ctext_signed.as_ptr() as *const _, + ctext_signed as *const _, ctext_signed_bytes, &keyring, &public_keyring2, @@ -774,13 +773,15 @@ fn test_encryption_decryption() { valid_signatures.clear(); let plain = dc_pgp_pk_decrypt( - ctext_unsigned.as_ptr() as *const _, + ctext_unsigned as *const _, ctext_unsigned_bytes, &keyring, &public_keyring, Some(&mut valid_signatures), ) .unwrap(); + + free(ctext_unsigned as *mut _); assert_eq!(std::str::from_utf8(&plain).unwrap(), as_str(original_text),); valid_signatures.clear(); @@ -791,13 +792,15 @@ fn test_encryption_decryption() { public_keyring.add_ref(&public_key); let plain = dc_pgp_pk_decrypt( - ctext_signed.as_ptr() as *const _, + ctext_signed as *const _, ctext_signed_bytes, &keyring, &public_keyring, None, ) .unwrap(); + + free(ctext_signed as *mut _); assert_eq!(std::str::from_utf8(&plain).unwrap(), as_str(original_text),); } } @@ -820,14 +823,14 @@ struct TestContext { unsafe fn create_test_context() -> TestContext { let mut ctx = dc_context_new(Some(cb), std::ptr::null_mut(), std::ptr::null_mut()); let dir = tempdir().unwrap(); - let dbfile = CString::new(dir.path().join("db.sqlite").to_str().unwrap()).unwrap(); + let dbfile = to_cstring(dir.path().join("db.sqlite").to_str().unwrap()); assert_eq!( - dc_open(&mut ctx, dbfile.as_ptr(), std::ptr::null()), + dc_open(&mut ctx, dbfile, std::ptr::null()), 1, "Failed to open {}", - as_str(dbfile.as_ptr() as *const libc::c_char) + as_str(dbfile as *const libc::c_char) ); - + free(dbfile as *mut _); TestContext { ctx: ctx, dir: dir } } @@ -1012,8 +1015,9 @@ fn test_wrong_db() { let dbfile = dir.path().join("db.sqlite"); std::fs::write(&dbfile, b"123").unwrap(); - let dbfile_c = CString::new(dbfile.to_str().unwrap()).unwrap(); - let res = dc_open(&mut ctx, dbfile_c.as_ptr(), std::ptr::null()); + let dbfile_c = to_cstring(dbfile.to_str().unwrap()); + let res = dc_open(&mut ctx, dbfile_c, std::ptr::null()); + free(dbfile_c as *mut _); assert_eq!(res, 0); } }