refactor(message): remove remaining unsafe and c types

This commit is contained in:
dignifiedquire
2019-09-21 14:58:35 +02:00
committed by holger krekel
parent 987f12740e
commit 0bdcc3d616
5 changed files with 93 additions and 146 deletions

View File

@@ -1172,7 +1172,7 @@ pub unsafe extern "C" fn dc_get_msg_info(
} }
let ffi_context = &*context; let ffi_context = &*context;
ffi_context ffi_context
.with_inner(|ctx| message::get_msg_info(ctx, msg_id)) .with_inner(|ctx| message::get_msg_info(ctx, msg_id).strdup())
.unwrap_or_else(|_| ptr::null_mut()) .unwrap_or_else(|_| ptr::null_mut())
} }
@@ -1187,7 +1187,11 @@ pub unsafe extern "C" fn dc_get_mime_headers(
} }
let ffi_context = &*context; let ffi_context = &*context;
ffi_context ffi_context
.with_inner(|ctx| message::get_mime_headers(ctx, msg_id)) .with_inner(|ctx| {
message::get_mime_headers(ctx, msg_id)
.map(|s| s.strdup())
.unwrap_or_else(|| ptr::null_mut())
})
.unwrap_or_else(|_| ptr::null_mut()) .unwrap_or_else(|_| ptr::null_mut())
} }
@@ -1202,8 +1206,11 @@ pub unsafe extern "C" fn dc_delete_msgs(
return; return;
} }
let ffi_context = &*context; let ffi_context = &*context;
let ids = std::slice::from_raw_parts(msg_ids, msg_cnt as usize);
ffi_context ffi_context
.with_inner(|ctx| message::delete_msgs(ctx, msg_ids, msg_cnt)) .with_inner(|ctx| message::delete_msgs(ctx, ids))
.unwrap_or(()) .unwrap_or(())
} }
@@ -1250,9 +1257,11 @@ pub unsafe extern "C" fn dc_markseen_msgs(
eprintln!("ignoring careless call to dc_markseen_msgs()"); eprintln!("ignoring careless call to dc_markseen_msgs()");
return; return;
} }
let ids = std::slice::from_raw_parts(msg_ids, msg_cnt as usize);
let ffi_context = &*context; let ffi_context = &*context;
ffi_context ffi_context
.with_inner(|ctx| message::markseen_msgs(ctx, msg_ids, msg_cnt as usize)) .with_inner(|ctx| message::markseen_msgs(ctx, ids))
.ok(); .ok();
} }
@@ -1267,9 +1276,12 @@ pub unsafe extern "C" fn dc_star_msgs(
eprintln!("ignoring careless call to dc_star_msgs()"); eprintln!("ignoring careless call to dc_star_msgs()");
return; return;
} }
let ids = std::slice::from_raw_parts(msg_ids, msg_cnt as usize);
let ffi_context = &*context; let ffi_context = &*context;
ffi_context ffi_context
.with_inner(|ctx| message::star_msgs(ctx, msg_ids, msg_cnt, star)) .with_inner(|ctx| message::star_msgs(ctx, ids, star == 1))
.ok(); .ok();
} }

View File

@@ -899,7 +899,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E
ensure!(!arg1.is_empty(), "Argument <msg-id> missing."); ensure!(!arg1.is_empty(), "Argument <msg-id> missing.");
let id = arg1.parse()?; let id = arg1.parse()?;
let res = message::get_msg_info(context, id); let res = message::get_msg_info(context, id);
println!("{}", as_str(res)); println!("{}", res);
} }
"listfresh" => { "listfresh" => {
let msglist = context.get_fresh_msgs(); let msglist = context.get_fresh_msgs();
@@ -922,24 +922,19 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E
ensure!(!arg1.is_empty(), "Argument <msg-id> missing."); ensure!(!arg1.is_empty(), "Argument <msg-id> missing.");
let mut msg_ids = [0; 1]; let mut msg_ids = [0; 1];
msg_ids[0] = arg1.parse()?; msg_ids[0] = arg1.parse()?;
message::markseen_msgs(context, msg_ids.as_mut_ptr(), 1); message::markseen_msgs(context, &msg_ids);
} }
"star" | "unstar" => { "star" | "unstar" => {
ensure!(!arg1.is_empty(), "Argument <msg-id> missing."); ensure!(!arg1.is_empty(), "Argument <msg-id> missing.");
let mut msg_ids = [0; 1]; let mut msg_ids = [0; 1];
msg_ids[0] = arg1.parse()?; msg_ids[0] = arg1.parse()?;
message::star_msgs( message::star_msgs(context, &msg_ids, arg0 == "star");
context,
msg_ids.as_mut_ptr(),
1,
if arg0 == "star" { 1 } else { 0 },
);
} }
"delmsg" => { "delmsg" => {
ensure!(!arg1.is_empty(), "Argument <msg-id> missing."); ensure!(!arg1.is_empty(), "Argument <msg-id> missing.");
let mut ids = [0; 1]; let mut ids = [0; 1];
ids[0] = arg1.parse()?; ids[0] = arg1.parse()?;
message::delete_msgs(context, ids.as_mut_ptr(), 1); message::delete_msgs(context, &ids);
} }
"listcontacts" | "contacts" | "listverified" => { "listcontacts" | "contacts" | "listverified" => {
let contacts = Contact::get_all( let contacts = Contact::get_all(

View File

@@ -351,20 +351,13 @@ unsafe fn add_parts(
// check, if the mail is already in our database - if so, just update the folder/uid // check, if the mail is already in our database - if so, just update the folder/uid
// (if the mail was moved around) and finish. (we may get a mail twice eg. if it is // (if the mail was moved around) and finish. (we may get a mail twice eg. if it is
// moved between folders. make sure, this check is done eg. before securejoin-processing) */ // moved between folders. make sure, this check is done eg. before securejoin-processing) */
let mut old_server_folder = std::ptr::null_mut(); if let Ok((old_server_folder, old_server_uid, _)) =
let mut old_server_uid = 0; message::rfc724_mid_exists(context, &rfc724_mid)
{
if 0 != message::rfc724_mid_exists( if old_server_folder != server_folder.as_ref() || old_server_uid != server_uid {
context,
&rfc724_mid,
&mut old_server_folder,
&mut old_server_uid,
) {
if as_str(old_server_folder) != server_folder.as_ref() || old_server_uid != server_uid {
message::update_server_uid(context, &rfc724_mid, server_folder.as_ref(), server_uid); message::update_server_uid(context, &rfc724_mid, server_folder.as_ref(), server_uid);
} }
free(old_server_folder.cast());
cleanup(mime_in_reply_to, mime_references); cleanup(mime_in_reply_to, mime_references);
bail!("Message already in DB"); bail!("Message already in DB");
} }
@@ -840,7 +833,7 @@ unsafe fn handle_reports(
let mut chat_id_0 = 0; let mut chat_id_0 = 0;
let mut msg_id = 0; let mut msg_id = 0;
if 0 != message::mdn_from_ext( if message::mdn_from_ext(
context, context,
from_id, from_id,
as_str(rfc724_mid_0), as_str(rfc724_mid_0),

View File

@@ -1,5 +1,4 @@
use std::net; use std::net;
use std::ptr;
use std::sync::{ use std::sync::{
atomic::{AtomicBool, Ordering}, atomic::{AtomicBool, Ordering},
Arc, Condvar, Mutex, RwLock, Arc, Condvar, Mutex, RwLock,
@@ -9,7 +8,6 @@ use std::time::{Duration, SystemTime};
use crate::constants::*; use crate::constants::*;
use crate::context::Context; use crate::context::Context;
use crate::dc_receive_imf::dc_receive_imf; use crate::dc_receive_imf::dc_receive_imf;
use crate::dc_tools::*;
use crate::events::Event; use crate::events::Event;
use crate::job::{job_add, Action}; use crate::job::{job_add, Action};
use crate::login_param::LoginParam; use crate::login_param::LoginParam;
@@ -822,7 +820,7 @@ impl Imap {
.message_id .message_id
.expect("missing message id"); .expect("missing message id");
if 0 == unsafe { precheck_imf(context, &message_id, folder.as_ref(), cur_uid) } { if !precheck_imf(context, &message_id, folder.as_ref(), cur_uid) {
// check passed, go fetch the rest // check passed, go fetch the rest
if self.fetch_single_msg(context, &folder, cur_uid) == 0 { if self.fetch_single_msg(context, &folder, cur_uid) == 0 {
info!( info!(
@@ -1641,39 +1639,28 @@ fn get_folder_meaning(folder_name: &imap::types::Name) -> FolderMeaning {
} }
} }
unsafe fn precheck_imf( fn precheck_imf(context: &Context, rfc724_mid: &str, server_folder: &str, server_uid: u32) -> bool {
context: &Context, let mut rfc724_mid_exists = false;
rfc724_mid: &str, let mut mark_seen = false;
server_folder: &str,
server_uid: u32, if let Ok((old_server_folder, old_server_uid, msg_id)) =
) -> libc::c_int { message::rfc724_mid_exists(context, &rfc724_mid)
let mut rfc724_mid_exists: libc::c_int = 0i32; {
let msg_id: u32; rfc724_mid_exists = true;
let mut old_server_folder: *mut libc::c_char = ptr::null_mut();
let mut old_server_uid: u32 = 0i32 as u32; if old_server_folder.is_empty() && old_server_uid == 0 {
let mut mark_seen: libc::c_int = 0i32;
msg_id = message::rfc724_mid_exists(
context,
&rfc724_mid,
&mut old_server_folder,
&mut old_server_uid,
);
if msg_id != 0i32 as libc::c_uint {
rfc724_mid_exists = 1i32;
if *old_server_folder.offset(0isize) as libc::c_int == 0i32
&& old_server_uid == 0i32 as libc::c_uint
{
info!(context, "[move] detected bbc-self {}", rfc724_mid,); info!(context, "[move] detected bbc-self {}", rfc724_mid,);
mark_seen = 1i32 mark_seen = true;
} else if as_str(old_server_folder) != server_folder { } else if old_server_folder != server_folder {
info!(context, "[move] detected moved message {}", rfc724_mid,); info!(context, "[move] detected moved message {}", rfc724_mid,);
update_msg_move_state(context, &rfc724_mid, MoveState::Stay); update_msg_move_state(context, &rfc724_mid, MoveState::Stay);
} }
if as_str(old_server_folder) != server_folder || old_server_uid != server_uid { if old_server_folder != server_folder || old_server_uid != server_uid {
update_server_uid(context, &rfc724_mid, server_folder, server_uid); update_server_uid(context, &rfc724_mid, server_folder, server_uid);
} }
context.do_heuristics_moves(server_folder, msg_id); context.do_heuristics_moves(server_folder, msg_id);
if 0 != mark_seen {
if mark_seen {
job_add( job_add(
context, context,
Action::MarkseenMsgOnImap, Action::MarkseenMsgOnImap,
@@ -1683,6 +1670,6 @@ unsafe fn precheck_imf(
); );
} }
} }
libc::free(old_server_folder as *mut libc::c_void);
rfc724_mid_exists rfc724_mid_exists
} }

View File

@@ -1,4 +1,3 @@
use std::ffi::CString;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::ptr; use std::ptr;
@@ -355,9 +354,9 @@ impl Message {
if let Some(mut buf) = dc_read_file_safe(context, filename) { if let Some(mut buf) = dc_read_file_safe(context, filename) {
unsafe { unsafe {
// just a pointer inside buf, MUST NOT be free()'d // just a pointer inside buf, MUST NOT be free()'d
let mut buf_headerline: *const libc::c_char = ptr::null(); let mut buf_headerline = ptr::null();
// just a pointer inside buf, MUST NOT be free()'d // just a pointer inside buf, MUST NOT be free()'d
let mut buf_setupcodebegin: *const libc::c_char = ptr::null(); let mut buf_setupcodebegin = ptr::null();
if dc_split_armored_data( if dc_split_armored_data(
buf.as_mut_ptr().cast(), buf.as_mut_ptr().cast(),
@@ -534,12 +533,12 @@ impl Lot {
} }
} }
pub unsafe fn get_msg_info(context: &Context, msg_id: u32) -> *mut libc::c_char { pub fn get_msg_info(context: &Context, msg_id: u32) -> String {
let mut ret = String::new(); let mut ret = String::new();
let msg = Message::load_from_db(context, msg_id); let msg = Message::load_from_db(context, msg_id);
if msg.is_err() { if msg.is_err() {
return ptr::null_mut(); return ret;
} }
let msg = msg.unwrap(); let msg = msg.unwrap();
@@ -552,7 +551,7 @@ pub unsafe fn get_msg_info(context: &Context, msg_id: u32) -> *mut libc::c_char
if rawtxt.is_none() { if rawtxt.is_none() {
ret += &format!("Cannot load message #{}.", msg_id as usize); ret += &format!("Cannot load message #{}.", msg_id as usize);
return ret.strdup(); return ret;
} }
let rawtxt = rawtxt.unwrap(); let rawtxt = rawtxt.unwrap();
let rawtxt = dc_truncate(rawtxt.trim(), 100000, false); let rawtxt = dc_truncate(rawtxt.trim(), 100000, false);
@@ -579,7 +578,7 @@ pub unsafe fn get_msg_info(context: &Context, msg_id: u32) -> *mut libc::c_char
if msg.from_id == 2 || msg.to_id == 2 { if msg.from_id == 2 || msg.to_id == 2 {
// device-internal message, no further details needed // device-internal message, no further details needed
return ret.strdup(); return ret;
} }
if let Ok(rows) = context.sql.query_map( if let Ok(rows) = context.sql.query_map(
@@ -671,7 +670,7 @@ pub unsafe fn get_msg_info(context: &Context, msg_id: u32) -> *mut libc::c_char
} }
} }
ret.strdup() ret
} }
pub fn guess_msgtype_from_suffix(path: &Path) -> Option<(Viewtype, &str)> { pub fn guess_msgtype_from_suffix(path: &Path) -> Option<(Viewtype, &str)> {
@@ -692,39 +691,27 @@ pub fn guess_msgtype_from_suffix(path: &Path) -> Option<(Viewtype, &str)> {
KNOWN.get(extension).map(|x| *x) KNOWN.get(extension).map(|x| *x)
} }
pub unsafe fn get_mime_headers(context: &Context, msg_id: u32) -> *mut libc::c_char { pub fn get_mime_headers(context: &Context, msg_id: u32) -> Option<String> {
let headers: Option<String> = context.sql.query_get_value( context.sql.query_get_value(
context, context,
"SELECT mime_headers FROM msgs WHERE id=?;", "SELECT mime_headers FROM msgs WHERE id=?;",
params![msg_id as i32], params![msg_id as i32],
); )
if let Some(headers) = headers {
let h = CString::yolo(headers);
dc_strdup_keep_null(h.as_ptr())
} else {
std::ptr::null_mut()
}
} }
pub unsafe fn delete_msgs(context: &Context, msg_ids: *const u32, msg_cnt: libc::c_int) { pub fn delete_msgs(context: &Context, msg_ids: &[u32]) {
if msg_ids.is_null() || msg_cnt <= 0i32 { for msg_id in msg_ids.into_iter() {
return; update_msg_chat_id(context, *msg_id, DC_CHAT_ID_TRASH);
}
let mut i: libc::c_int = 0i32;
while i < msg_cnt {
update_msg_chat_id(context, *msg_ids.offset(i as isize), 3i32 as u32);
job_add( job_add(
context, context,
Action::DeleteMsgOnImap, Action::DeleteMsgOnImap,
*msg_ids.offset(i as isize) as libc::c_int, *msg_id as libc::c_int,
Params::new(), Params::new(),
0, 0,
); );
i += 1
} }
if 0 != msg_cnt { if !msg_ids.is_empty() {
context.call_cb(Event::MsgsChanged { context.call_cb(Event::MsgsChanged {
chat_id: 0, chat_id: 0,
msg_id: 0, msg_id: 0,
@@ -744,17 +731,17 @@ fn update_msg_chat_id(context: &Context, msg_id: u32, chat_id: u32) -> bool {
.is_ok() .is_ok()
} }
pub fn markseen_msgs(context: &Context, msg_ids: *const u32, msg_cnt: usize) -> bool { pub fn markseen_msgs(context: &Context, msg_ids: &[u32]) -> bool {
if msg_ids.is_null() || msg_cnt <= 0 { if msg_ids.is_empty() {
return false; return false;
} }
let msgs = context.sql.prepare( let msgs = context.sql.prepare(
"SELECT m.state, c.blocked FROM msgs m LEFT JOIN chats c ON c.id=m.chat_id WHERE m.id=? AND m.chat_id>9", "SELECT m.state, c.blocked FROM msgs m LEFT JOIN chats c ON c.id=m.chat_id WHERE m.id=? AND m.chat_id>9",
|mut stmt, _| { |mut stmt, _| {
let mut res = Vec::with_capacity(msg_cnt); let mut res = Vec::with_capacity(msg_ids.len());
for i in 0..msg_cnt { for id in msg_ids.into_iter() {
let id = unsafe { *msg_ids.offset(i as isize) }; let query_res = stmt.query_row(params![*id as i32], |row| {
let query_res = stmt.query_row(params![id as i32], |row| {
Ok((row.get::<_, MessageState>(0)?, row.get::<_, Option<Blocked>>(1)?.unwrap_or_default())) Ok((row.get::<_, MessageState>(0)?, row.get::<_, Option<Blocked>>(1)?.unwrap_or_default()))
}); });
if let Err(rusqlite::Error::QueryReturnedNoRows) = query_res { if let Err(rusqlite::Error::QueryReturnedNoRows) = query_res {
@@ -763,6 +750,7 @@ pub fn markseen_msgs(context: &Context, msg_ids: *const u32, msg_cnt: usize) ->
let (state, blocked) = query_res?; let (state, blocked) = query_res?;
res.push((id, state, blocked)); res.push((id, state, blocked));
} }
Ok(res) Ok(res)
} }
); );
@@ -777,20 +765,20 @@ pub fn markseen_msgs(context: &Context, msg_ids: *const u32, msg_cnt: usize) ->
for (id, curr_state, curr_blocked) in msgs.into_iter() { for (id, curr_state, curr_blocked) in msgs.into_iter() {
if curr_blocked == Blocked::Not { if curr_blocked == Blocked::Not {
if curr_state == MessageState::InFresh || curr_state == MessageState::InNoticed { if curr_state == MessageState::InFresh || curr_state == MessageState::InNoticed {
update_msg_state(context, id, MessageState::InSeen); update_msg_state(context, *id, MessageState::InSeen);
info!(context, "Seen message #{}.", id); info!(context, "Seen message #{}.", id);
job_add( job_add(
context, context,
Action::MarkseenMsgOnImap, Action::MarkseenMsgOnImap,
id as i32, *id as i32,
Params::new(), Params::new(),
0, 0,
); );
send_event = true; send_event = true;
} }
} else if curr_state == MessageState::InFresh { } else if curr_state == MessageState::InFresh {
update_msg_state(context, id, MessageState::InNoticed); update_msg_state(context, *id, MessageState::InNoticed);
send_event = true; send_event = true;
} }
} }
@@ -815,20 +803,15 @@ pub fn update_msg_state(context: &Context, msg_id: u32, state: MessageState) ->
.is_ok() .is_ok()
} }
pub fn star_msgs( pub fn star_msgs(context: &Context, msg_ids: &[u32], star: bool) -> bool {
context: &Context, if msg_ids.is_empty() {
msg_ids: *const u32,
msg_cnt: libc::c_int,
star: libc::c_int,
) -> bool {
if msg_ids.is_null() || msg_cnt <= 0 || star != 0 && star != 1 {
return false; return false;
} }
context context
.sql .sql
.prepare("UPDATE msgs SET starred=? WHERE id=?;", |mut stmt, _| { .prepare("UPDATE msgs SET starred=? WHERE id=?;", |mut stmt, _| {
for i in 0..msg_cnt { for msg_id in msg_ids.into_iter() {
stmt.execute(params![star, unsafe { *msg_ids.offset(i as isize) as i32 }])?; stmt.execute(params![star as i32, *msg_id as i32])?;
} }
Ok(()) Ok(())
}) })
@@ -964,26 +947,20 @@ pub fn set_msg_failed(context: &Context, msg_id: u32, error: Option<impl AsRef<s
} }
} }
///returns 1 if an event should be send /// returns true if an event should be send
pub unsafe fn mdn_from_ext( pub fn mdn_from_ext(
context: &Context, context: &Context,
from_id: u32, from_id: u32,
rfc724_mid: &str, rfc724_mid: &str,
timestamp_sent: i64, timestamp_sent: i64,
ret_chat_id: *mut u32, ret_chat_id: &mut u32,
ret_msg_id: *mut u32, ret_msg_id: &mut u32,
) -> libc::c_int { ) -> bool {
if from_id <= 9 if from_id <= 9 || rfc724_mid.is_empty() || *ret_chat_id != 0 || *ret_msg_id != 0 {
|| rfc724_mid.is_empty() return false;
|| ret_chat_id.is_null()
|| ret_msg_id.is_null()
|| *ret_chat_id != 0
|| *ret_msg_id != 0
{
return 0;
} }
let mut read_by_all = 0; let mut read_by_all = false;
if let Ok((msg_id, chat_id, chat_type, msg_state)) = context.sql.query_row( if let Ok((msg_id, chat_id, chat_type, msg_state)) = context.sql.query_row(
"SELECT m.id, c.id, c.type, m.state FROM msgs m \ "SELECT m.id, c.id, c.type, m.state FROM msgs m \
@@ -1025,7 +1002,7 @@ pub unsafe fn mdn_from_ext(
// Normal chat? that's quite easy. // Normal chat? that's quite easy.
if chat_type == Chattype::Single { if chat_type == Chattype::Single {
update_msg_state(context, *ret_msg_id, MessageState::OutMdnRcvd); update_msg_state(context, *ret_msg_id, MessageState::OutMdnRcvd);
read_by_all = 1; read_by_all = true;
} else { } else {
// send event about new state // send event about new state
let ist_cnt: i32 = context let ist_cnt: i32 = context
@@ -1052,7 +1029,7 @@ pub unsafe fn mdn_from_ext(
let soll_cnt = (chat::get_chat_contact_cnt(context, *ret_chat_id) + 1) / 2; let soll_cnt = (chat::get_chat_contact_cnt(context, *ret_chat_id) + 1) / 2;
if ist_cnt >= soll_cnt { if ist_cnt >= soll_cnt {
update_msg_state(context, *ret_msg_id, MessageState::OutMdnRcvd); update_msg_state(context, *ret_msg_id, MessageState::OutMdnRcvd);
read_by_all = 1; read_by_all = true;
} // else wait for more receipts } // else wait for more receipts
} }
} }
@@ -1109,40 +1086,23 @@ pub fn rfc724_mid_cnt(context: &Context, rfc724_mid: &str) -> libc::c_int {
} }
} }
pub fn rfc724_mid_exists( pub(crate) fn rfc724_mid_exists(
context: &Context, context: &Context,
rfc724_mid: &str, rfc724_mid: &str,
ret_server_folder: *mut *mut libc::c_char, ) -> Result<(String, u32, u32), Error> {
ret_server_uid: *mut u32, ensure!(!rfc724_mid.is_empty(), "empty rfc724_mid");
) -> u32 {
if rfc724_mid.is_empty() { context.sql.query_row(
return 0;
}
match context.sql.query_row(
"SELECT server_folder, server_uid, id FROM msgs WHERE rfc724_mid=?", "SELECT server_folder, server_uid, id FROM msgs WHERE rfc724_mid=?",
&[rfc724_mid], &[rfc724_mid],
|row| { |row| {
if !ret_server_folder.is_null() { let server_folder = row.get::<_, Option<String>>(0)?.unwrap_or_default();
unsafe { *ret_server_folder = row.get::<_, String>(0)?.strdup() }; let server_uid = row.get(1)?;
} let msg_id = row.get(2)?;
if !ret_server_uid.is_null() {
unsafe { *ret_server_uid = row.get(1)? };
}
row.get(2)
},
) {
Ok(res) => res,
Err(_err) => {
if !ret_server_folder.is_null() {
unsafe { *ret_server_folder = ptr::null_mut() };
}
if !ret_server_uid.is_null() {
unsafe { *ret_server_uid = 0 };
}
0 Ok((server_folder, server_uid, msg_id))
} },
} )
} }
pub fn update_server_uid( pub fn update_server_uid(