refactor(message): remove unsafe and c types from the Message api

This commit is contained in:
dignifiedquire
2019-09-21 14:32:54 +02:00
committed by holger krekel
parent 1265016a55
commit 987f12740e
6 changed files with 77 additions and 101 deletions

View File

@@ -2282,7 +2282,7 @@ pub unsafe extern "C" fn dc_msg_get_text(msg: *mut dc_msg_t) -> *mut libc::c_cha
return dc_strdup(ptr::null()); return dc_strdup(ptr::null());
} }
let ffi_msg = &*msg; let ffi_msg = &*msg;
ffi_msg.message.get_text() ffi_msg.message.get_text().unwrap_or_default().strdup()
} }
#[no_mangle] #[no_mangle]
@@ -2312,7 +2312,7 @@ pub unsafe extern "C" fn dc_msg_get_filename(msg: *mut dc_msg_t) -> *mut libc::c
return dc_strdup(ptr::null()); return dc_strdup(ptr::null());
} }
let ffi_msg = &*msg; let ffi_msg = &*msg;
ffi_msg.message.get_filename() ffi_msg.message.get_filename().unwrap_or_default().strdup()
} }
#[no_mangle] #[no_mangle]
@@ -2420,7 +2420,8 @@ pub unsafe extern "C" fn dc_msg_get_summarytext(
.message .message
.get_summarytext(ctx, approx_characters.try_into().unwrap()) .get_summarytext(ctx, approx_characters.try_into().unwrap())
}) })
.unwrap_or_else(|_| "".strdup()) .unwrap_or_default()
.strdup()
} }
#[no_mangle] #[no_mangle]
@@ -2430,7 +2431,7 @@ pub unsafe extern "C" fn dc_msg_has_deviating_timestamp(msg: *mut dc_msg_t) -> l
return 0; return 0;
} }
let ffi_msg = &*msg; let ffi_msg = &*msg;
ffi_msg.message.has_deviating_timestamp() ffi_msg.message.has_deviating_timestamp().into()
} }
#[no_mangle] #[no_mangle]
@@ -2512,8 +2513,9 @@ pub unsafe extern "C" fn dc_msg_get_setupcodebegin(msg: *mut dc_msg_t) -> *mut l
let ffi_msg = &*msg; let ffi_msg = &*msg;
let ffi_context = &*ffi_msg.context; let ffi_context = &*ffi_msg.context;
ffi_context ffi_context
.with_inner(|ctx| ffi_msg.message.get_setupcodebegin(ctx)) .with_inner(|ctx| ffi_msg.message.get_setupcodebegin(ctx).unwrap_or_default())
.unwrap_or_else(|_| "".strdup()) .unwrap_or_default()
.strdup()
} }
#[no_mangle] #[no_mangle]
@@ -2524,7 +2526,7 @@ pub unsafe extern "C" fn dc_msg_set_text(msg: *mut dc_msg_t, text: *mut libc::c_
} }
let ffi_msg = &mut *msg; let ffi_msg = &mut *msg;
// TODO: {text} equal to NULL is treated as "", which is strange. Does anyone rely on it? // TODO: {text} equal to NULL is treated as "", which is strange. Does anyone rely on it?
ffi_msg.message.set_text(text) ffi_msg.message.set_text(as_opt_str(text).map(Into::into))
} }
#[no_mangle] #[no_mangle]
@@ -2533,12 +2535,12 @@ pub unsafe extern "C" fn dc_msg_set_file(
file: *mut libc::c_char, file: *mut libc::c_char,
filemime: *mut libc::c_char, filemime: *mut libc::c_char,
) { ) {
if msg.is_null() { if msg.is_null() || file.is_null() {
eprintln!("ignoring careless call to dc_msg_set_file()"); eprintln!("ignoring careless call to dc_msg_set_file()");
return; return;
} }
let ffi_msg = &mut *msg; let ffi_msg = &mut *msg;
ffi_msg.message.set_file(file, filemime) ffi_msg.message.set_file(as_str(file), as_opt_str(filemime))
} }
#[no_mangle] #[no_mangle]

View File

@@ -238,7 +238,7 @@ unsafe fn log_msg(context: &Context, prefix: impl AsRef<str>, msg: &Message) {
if msg.has_location() { "📍" } else { "" }, if msg.has_location() { "📍" } else { "" },
&contact_name, &contact_name,
contact_id, contact_id,
as_str(msgtext), msgtext.unwrap_or_default(),
if msg.is_starred() { "" } else { "" }, if msg.is_starred() { "" } else { "" },
if msg.get_from_id() == 1 as libc::c_uint { if msg.get_from_id() == 1 as libc::c_uint {
"" ""
@@ -253,7 +253,6 @@ unsafe fn log_msg(context: &Context, prefix: impl AsRef<str>, msg: &Message) {
statestr, statestr,
&temp2, &temp2,
); );
free(msgtext as *mut libc::c_void);
} }
unsafe fn log_msglist(context: &Context, msglist: &Vec<u32>) -> Result<(), Error> { unsafe fn log_msglist(context: &Context, msglist: &Vec<u32>) -> Result<(), Error> {
@@ -469,9 +468,8 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E
println!( println!(
"The setup code for setup message Msg#{} starts with: {}", "The setup code for setup message Msg#{} starts with: {}",
msg_id, msg_id,
as_str(setupcodebegin), setupcodebegin.unwrap_or_default(),
); );
free(setupcodebegin as *mut libc::c_void);
} else { } else {
bail!("Msg#{} is no setup message.", msg_id,); bail!("Msg#{} is no setup message.", msg_id,);
} }
@@ -826,9 +824,9 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E
} else { } else {
Viewtype::File Viewtype::File
}); });
msg.set_file(arg1_c, ptr::null()); msg.set_file(arg1, None);
if !arg2.is_empty() { if !arg2.is_empty() {
msg.set_text(arg2_c); msg.set_text(Some(arg2.to_string()));
} }
chat::send_msg(context, sel_chat.as_ref().unwrap().get_id(), &mut msg)?; chat::send_msg(context, sel_chat.as_ref().unwrap().get_id(), &mut msg)?;
} }
@@ -851,7 +849,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E
if !arg1.is_empty() { if !arg1.is_empty() {
let mut draft = Message::new(Viewtype::Text); let mut draft = Message::new(Viewtype::Text);
draft.set_text(arg1_c); draft.set_text(Some(arg1.to_string()));
chat::set_draft( chat::set_draft(
context, context,
sel_chat.as_ref().unwrap().get_id(), sel_chat.as_ref().unwrap().get_id(),

View File

@@ -1,4 +1,3 @@
use std::ffi::CString;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use crate::chatlist::*; use crate::chatlist::*;
@@ -1277,9 +1276,7 @@ pub unsafe fn create_group_chat(
) -> Result<u32, Error> { ) -> Result<u32, Error> {
ensure!(!chat_name.as_ref().is_empty(), "Invalid chat name"); ensure!(!chat_name.as_ref().is_empty(), "Invalid chat name");
let draft_txt = let draft_txt = context.stock_string_repl_str(StockMessage::NewGroupDraft, &chat_name);
CString::new(context.stock_string_repl_str(StockMessage::NewGroupDraft, &chat_name))
.unwrap();
let grpid = dc_create_id(); let grpid = dc_create_id();
sql::execute( sql::execute(
@@ -1302,7 +1299,7 @@ pub unsafe fn create_group_chat(
if chat_id != 0 { if chat_id != 0 {
if add_to_chat_contacts_table(context, chat_id, 1) { if add_to_chat_contacts_table(context, chat_id, 1) {
let mut draft_msg = Message::new(Viewtype::Text); let mut draft_msg = Message::new(Viewtype::Text);
draft_msg.set_text(draft_txt.as_ptr()); draft_msg.set_text(Some(draft_txt));
set_draft_raw(context, chat_id, &mut draft_msg); set_draft_raw(context, chat_id, &mut draft_msg);
} }
@@ -1911,12 +1908,12 @@ mod tests {
let t = dummy_context(); let t = dummy_context();
let chat_id = create_by_contact_id(&t.ctx, DC_CONTACT_ID_SELF).unwrap(); let chat_id = create_by_contact_id(&t.ctx, DC_CONTACT_ID_SELF).unwrap();
let mut msg = Message::new(Viewtype::Text); let mut msg = Message::new(Viewtype::Text);
msg.set_text(b"hello\x00" as *const u8 as *const libc::c_char); msg.set_text(Some("hello".to_string()));
set_draft(&t.ctx, chat_id, Some(&mut msg)); set_draft(&t.ctx, chat_id, Some(&mut msg));
let draft = get_draft(&t.ctx, chat_id).unwrap().unwrap(); let draft = get_draft(&t.ctx, chat_id).unwrap().unwrap();
let msg_text = msg.get_text(); let msg_text = msg.get_text();
let draft_text = draft.get_text(); let draft_text = draft.get_text();
assert_eq!(as_str(msg_text), as_str(draft_text)); assert_eq!(msg_text, draft_text);
} }
} }
} }

View File

@@ -122,7 +122,7 @@ const DC_MAX_GET_INFO_LEN: usize = 100000;
pub const DC_CONTACT_ID_UNDEFINED: u32 = 0; pub const DC_CONTACT_ID_UNDEFINED: u32 = 0;
pub const DC_CONTACT_ID_SELF: u32 = 1; pub const DC_CONTACT_ID_SELF: u32 = 1;
const DC_CONTACT_ID_DEVICE: u32 = 2; pub const DC_CONTACT_ID_DEVICE: u32 = 2;
pub const DC_CONTACT_ID_LAST_SPECIAL: u32 = 9; pub const DC_CONTACT_ID_LAST_SPECIAL: u32 = 9;
pub const DC_CREATE_MVBOX: usize = 1; pub const DC_CREATE_MVBOX: usize = 1;

View File

@@ -917,7 +917,7 @@ pub unsafe fn dc_mimefactory_render(context: &Context, factory: &mut MimeFactory
.stock_str(StockMessage::EncryptedMsg) .stock_str(StockMessage::EncryptedMsg)
.into_owned() .into_owned()
} else { } else {
to_string(factory.msg.get_summarytext(context, 32)) factory.msg.get_summarytext(context, 32)
}; };
let p2 = factory let p2 = factory
.context .context

View File

@@ -235,44 +235,35 @@ impl Message {
self.timestamp_sort self.timestamp_sort
} }
pub unsafe fn get_text(&self) -> *mut libc::c_char { pub fn get_text(&self) -> Option<String> {
if let Some(ref text) = self.text { self.text
dc_truncate(text, 30000, false).strdup() .as_ref()
} else { .map(|text| dc_truncate(text, 30000, false).to_string())
ptr::null_mut()
}
} }
#[allow(non_snake_case)] pub fn get_filename(&self) -> Option<String> {
pub unsafe fn get_filename(&self) -> *mut libc::c_char { self.param
let mut ret = ptr::null_mut(); .get(Param::File)
.and_then(|file| Path::new(file).file_name())
if let Some(file) = self.param.get(Param::File) { .map(|name| name.to_string_lossy().to_string())
ret = dc_get_filename(file);
}
if !ret.is_null() {
ret
} else {
dc_strdup(0 as *const libc::c_char)
}
} }
pub fn get_filebytes(&self, context: &Context) -> u64 { pub fn get_filebytes(&self, context: &Context) -> u64 {
if let Some(file) = self.param.get(Param::File) { self.param
return dc_get_filebytes(context, &file); .get(Param::File)
} .map(|file| dc_get_filebytes(context, &file))
0 .unwrap_or_default()
} }
pub fn get_width(&self) -> libc::c_int { pub fn get_width(&self) -> i32 {
self.param.get_int(Param::Width).unwrap_or_default() self.param.get_int(Param::Width).unwrap_or_default()
} }
pub fn get_height(&self) -> libc::c_int { pub fn get_height(&self) -> i32 {
self.param.get_int(Param::Height).unwrap_or_default() self.param.get_int(Param::Height).unwrap_or_default()
} }
pub fn get_duration(&self) -> libc::c_int { pub fn get_duration(&self) -> i32 {
self.param.get_int(Param::Duration).unwrap_or_default() self.param.get_int(Param::Duration).unwrap_or_default()
} }
@@ -286,13 +277,11 @@ impl Message {
let chat_loaded: Chat; let chat_loaded: Chat;
let chat = if let Some(chat) = chat { let chat = if let Some(chat) = chat {
chat chat
} else if let Ok(chat) = Chat::load_from_db(context, self.chat_id) {
chat_loaded = chat;
&chat_loaded
} else { } else {
if let Ok(chat) = Chat::load_from_db(context, self.chat_id) { return ret;
chat_loaded = chat;
&chat_loaded
} else {
return ret;
}
}; };
let contact = if self.from_id != DC_CONTACT_ID_SELF as libc::c_uint let contact = if self.from_id != DC_CONTACT_ID_SELF as libc::c_uint
@@ -308,11 +297,7 @@ impl Message {
ret ret
} }
pub unsafe fn get_summarytext( pub fn get_summarytext(&mut self, context: &Context, approx_characters: usize) -> String {
&mut self,
context: &Context,
approx_characters: usize,
) -> *mut libc::c_char {
get_summarytext_by_raw( get_summarytext_by_raw(
self.type_0, self.type_0,
self.text.as_ref(), self.text.as_ref(),
@@ -320,15 +305,14 @@ impl Message {
approx_characters, approx_characters,
context, context,
) )
.strdup()
} }
pub unsafe fn has_deviating_timestamp(&self) -> libc::c_int { pub fn has_deviating_timestamp(&self) -> bool {
let cnv_to_local = dc_gm2local_offset(); let cnv_to_local = dc_gm2local_offset();
let sort_timestamp = self.get_sort_timestamp() as i64 + cnv_to_local; let sort_timestamp = self.get_sort_timestamp() as i64 + cnv_to_local;
let send_timestamp = self.get_timestamp() as i64 + cnv_to_local; let send_timestamp = self.get_timestamp() as i64 + cnv_to_local;
(sort_timestamp / 86400 != send_timestamp / 86400) as libc::c_int sort_timestamp / 86400 != send_timestamp / 86400
} }
pub fn is_sent(&self) -> bool { pub fn is_sent(&self) -> bool {
@@ -345,8 +329,8 @@ impl Message {
pub fn is_info(&self) -> bool { pub fn is_info(&self) -> bool {
let cmd = self.param.get_cmd(); let cmd = self.param.get_cmd();
self.from_id == 2i32 as libc::c_uint self.from_id == DC_CONTACT_ID_DEVICE as libc::c_uint
|| self.to_id == 2i32 as libc::c_uint || self.to_id == DC_CONTACT_ID_DEVICE as libc::c_uint
|| cmd != SystemMessage::Unknown && cmd != SystemMessage::AutocryptSetupMessage || cmd != SystemMessage::Unknown && cmd != SystemMessage::AutocryptSetupMessage
} }
@@ -362,15 +346,19 @@ impl Message {
self.param.get_cmd() == SystemMessage::AutocryptSetupMessage self.param.get_cmd() == SystemMessage::AutocryptSetupMessage
} }
pub unsafe fn get_setupcodebegin(&self, context: &Context) -> *mut libc::c_char { pub fn get_setupcodebegin(&self, context: &Context) -> Option<String> {
// just a pointer inside buf, MUST NOT be free()'d
let mut buf_headerline: *const libc::c_char = ptr::null();
// just a pointer inside buf, MUST NOT be free()'d
let mut buf_setupcodebegin: *const libc::c_char = ptr::null();
let mut ret: *mut libc::c_char = ptr::null_mut();
if self.is_setupmessage() { if self.is_setupmessage() {
if let Some(filename) = self.get_file(context) { return None;
if let Some(mut buf) = dc_read_file_safe(context, filename) { }
if let Some(filename) = self.get_file(context) {
if let Some(mut buf) = dc_read_file_safe(context, filename) {
unsafe {
// just a pointer inside buf, MUST NOT be free()'d
let mut buf_headerline: *const libc::c_char = ptr::null();
// just a pointer inside buf, MUST NOT be free()'d
let mut buf_setupcodebegin: *const libc::c_char = ptr::null();
if dc_split_armored_data( if dc_split_armored_data(
buf.as_mut_ptr().cast(), buf.as_mut_ptr().cast(),
&mut buf_headerline, &mut buf_headerline,
@@ -383,50 +371,41 @@ impl Message {
) == 0 ) == 0
&& !buf_setupcodebegin.is_null() && !buf_setupcodebegin.is_null()
{ {
ret = dc_strdup(buf_setupcodebegin) return Some(to_string(buf_setupcodebegin));
} }
} }
} }
} }
if !ret.is_null() {
ret None
} else { }
dc_strdup(0 as *const libc::c_char)
pub fn set_text(&mut self, text: Option<String>) {
self.text = text;
}
pub fn set_file(&mut self, file: impl AsRef<str>, filemime: Option<&str>) {
self.param.set(Param::File, file);
if let Some(filemime) = filemime {
self.param.set(Param::MimeType, filemime);
} }
} }
pub fn set_text(&mut self, text: *const libc::c_char) { pub fn set_dimension(&mut self, width: i32, height: i32) {
self.text = if text.is_null() {
None
} else {
Some(to_string(text))
};
}
pub fn set_file(&mut self, file: *const libc::c_char, filemime: *const libc::c_char) {
if !file.is_null() {
self.param.set(Param::File, as_str(file));
}
if !filemime.is_null() {
self.param.set(Param::MimeType, as_str(filemime));
}
}
pub fn set_dimension(&mut self, width: libc::c_int, height: libc::c_int) {
self.param.set_int(Param::Width, width); self.param.set_int(Param::Width, width);
self.param.set_int(Param::Height, height); self.param.set_int(Param::Height, height);
} }
pub fn set_duration(&mut self, duration: libc::c_int) { pub fn set_duration(&mut self, duration: i32) {
self.param.set_int(Param::Duration, duration); self.param.set_int(Param::Duration, duration);
} }
pub fn latefiling_mediasize( pub fn latefiling_mediasize(
&mut self, &mut self,
context: &Context, context: &Context,
width: libc::c_int, width: i32,
height: libc::c_int, height: i32,
duration: libc::c_int, duration: i32,
) { ) {
if width > 0 && height > 0 { if width > 0 && height > 0 {
self.param.set_int(Param::Width, width); self.param.set_int(Param::Width, width);