get rid of unsafe and indirect return values for create_or_lookup.*group

This commit is contained in:
holger krekel
2019-12-04 10:08:50 +01:00
parent 609b5588fa
commit 56fc5a19ae

View File

@@ -414,20 +414,19 @@ fn add_parts(
Blocked::Deaddrop Blocked::Deaddrop
}; };
create_or_lookup_group( let (new_chat_id, chat_id_blocked) = create_or_lookup_group(
context, context,
&mut mime_parser, &mut mime_parser,
allow_creation, allow_creation,
create_blocked, create_blocked,
*from_id, *from_id,
to_ids, to_ids,
chat_id,
&mut chat_id_blocked,
)?; )?;
if 0 != *chat_id && Blocked::Not != chat_id_blocked && create_blocked == Blocked::Not { if new_chat_id != 0 && chat_id_blocked != Blocked::Not && create_blocked == Blocked::Not
chat::unblock(context, *chat_id); {
chat_id_blocked = Blocked::Not; chat::unblock(context, new_chat_id);
} }
*chat_id = new_chat_id;
} }
if *chat_id == 0 { if *chat_id == 0 {
@@ -507,20 +506,18 @@ fn add_parts(
if !to_ids.is_empty() { if !to_ids.is_empty() {
*to_id = to_ids[0]; *to_id = to_ids[0];
if *chat_id == 0 { if *chat_id == 0 {
create_or_lookup_group( let (new_chat_id, chat_id_blocked) = create_or_lookup_group(
context, context,
&mut mime_parser, &mut mime_parser,
allow_creation, allow_creation,
Blocked::Not, Blocked::Not,
*from_id, *from_id,
to_ids, to_ids,
chat_id,
&mut chat_id_blocked,
)?; )?;
if 0 != *chat_id && Blocked::Not != chat_id_blocked { if new_chat_id != 0 && chat_id_blocked != Blocked::Not {
chat::unblock(context, *chat_id); chat::unblock(context, *chat_id);
chat_id_blocked = Blocked::Not;
} }
*chat_id = new_chat_id;
} }
if *chat_id == 0 && 0 != allow_creation { if *chat_id == 0 && 0 != allow_creation {
let create_blocked = if 0 != msgrmsg && !Contact::is_blocked_load(context, *to_id) { let create_blocked = if 0 != msgrmsg && !Contact::is_blocked_load(context, *to_id) {
@@ -539,7 +536,6 @@ fn add_parts(
&& Blocked::Not == create_blocked && Blocked::Not == create_blocked
{ {
chat::unblock(context, *chat_id); chat::unblock(context, *chat_id);
chat_id_blocked = Blocked::Not;
} }
} }
} }
@@ -798,11 +794,8 @@ fn create_or_lookup_group(
create_blocked: Blocked, create_blocked: Blocked,
from_id: u32, from_id: u32,
to_ids: &mut Vec<u32>, to_ids: &mut Vec<u32>,
ret_chat_id: *mut u32, ) -> Result<(u32, Blocked)> {
ret_chat_id_blocked: &mut Blocked,
) -> Result<()> {
let group_explicitly_left: bool; let group_explicitly_left: bool;
let mut chat_id = 0;
let mut chat_id_blocked = Blocked::Not; let mut chat_id_blocked = Blocked::Not;
let mut grpid = "".to_string(); let mut grpid = "".to_string();
let mut grpname = None; let mut grpname = None;
@@ -815,18 +808,15 @@ fn create_or_lookup_group(
let mut X_MrGrpImageChanged = "".to_string(); let mut X_MrGrpImageChanged = "".to_string();
let mut better_msg: String = From::from(""); let mut better_msg: String = From::from("");
let cleanup = |ret_chat_id: *mut u32, let cleanup = |chat_id: u32, chat_id_blocked: Blocked| {
ret_chat_id_blocked: &mut Blocked, Ok((
chat_id: u32, chat_id,
chat_id_blocked: Blocked| { if 0 != chat_id {
if !ret_chat_id.is_null() {
unsafe { *ret_chat_id = chat_id };
}
*ret_chat_id_blocked = if 0 != chat_id {
chat_id_blocked chat_id_blocked
} else { } else {
Blocked::Not Blocked::Not
}; },
))
}; };
if mime_parser.is_system_message == SystemMessage::LocationStreamingEnabled { if mime_parser.is_system_message == SystemMessage::LocationStreamingEnabled {
@@ -857,18 +847,15 @@ fn create_or_lookup_group(
} }
if grpid.is_empty() { if grpid.is_empty() {
create_or_lookup_adhoc_group( let (new_chat_id, new_chat_id_blocked) = create_or_lookup_adhoc_group(
context, context,
mime_parser, mime_parser,
allow_creation, allow_creation,
create_blocked, create_blocked,
from_id, from_id,
to_ids, to_ids,
&mut chat_id,
&mut chat_id_blocked,
)?; )?;
cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked); return cleanup(new_chat_id, new_chat_id_blocked);
return Ok(());
} }
} }
} }
@@ -991,8 +978,7 @@ fn create_or_lookup_group(
} }
} }
if 0 == allow_creation { if 0 == allow_creation {
cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked); return cleanup(chat_id, chat_id_blocked);
return Ok(());
} }
chat_id = create_group_record( chat_id = create_group_record(
context, context,
@@ -1007,23 +993,21 @@ fn create_or_lookup_group(
// again, check chat_id // again, check chat_id
if chat_id <= DC_CHAT_ID_LAST_SPECIAL { if chat_id <= DC_CHAT_ID_LAST_SPECIAL {
chat_id = 0;
if group_explicitly_left { if group_explicitly_left {
chat_id = DC_CHAT_ID_TRASH; chat_id = DC_CHAT_ID_TRASH;
} else { } else {
create_or_lookup_adhoc_group( let (new_chat_id, new_chat_id_blocked) = create_or_lookup_adhoc_group(
context, context,
mime_parser, mime_parser,
allow_creation, allow_creation,
create_blocked, create_blocked,
from_id, from_id,
to_ids, to_ids,
&mut chat_id,
&mut chat_id_blocked,
)?; )?;
chat_id = new_chat_id;
chat_id_blocked = new_chat_id_blocked;
} }
cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked); return cleanup(chat_id, chat_id_blocked);
return Ok(());
} }
// execute group commands // execute group commands
@@ -1142,14 +1126,11 @@ fn create_or_lookup_group(
create_blocked, create_blocked,
from_id, from_id,
to_ids, to_ids,
&mut chat_id,
&mut chat_id_blocked,
)?; )?;
} }
} }
cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked); return cleanup(chat_id, chat_id_blocked);
Ok(())
} }
/// Handle groups for received messages /// Handle groups for received messages
@@ -1160,30 +1141,15 @@ fn create_or_lookup_adhoc_group(
create_blocked: Blocked, create_blocked: Blocked,
from_id: u32, from_id: u32,
to_ids: &mut Vec<u32>, to_ids: &mut Vec<u32>,
ret_chat_id: *mut u32, ) -> Result<(u32, Blocked)> {
ret_chat_id_blocked: &mut Blocked,
) -> Result<()> {
// if we're here, no grpid was found, check there is an existing ad-hoc // if we're here, no grpid was found, check there is an existing ad-hoc
// group matching the to-list or if we can create one // group matching the to-list or if we can create one
let mut chat_id = 0;
let mut chat_id_blocked = Blocked::Not;
let cleanup = |ret_chat_id: *mut u32, ensure!(!to_ids.is_empty(), "empty To-list");
ret_chat_id_blocked: &mut Blocked, ensure!(
chat_id: u32, !mime_parser.is_mailinglist_message(),
chat_id_blocked: Blocked| { "mailing-list message"
if !ret_chat_id.is_null() { );
unsafe { *ret_chat_id = chat_id };
}
*ret_chat_id_blocked = chat_id_blocked;
};
// build member list from the given ids
if to_ids.is_empty() || mime_parser.is_mailinglist_message() {
// too few contacts or a mailinglist
cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked);
return Ok(());
}
let mut member_ids = to_ids.clone(); let mut member_ids = to_ids.clone();
if !member_ids.contains(&from_id) { if !member_ids.contains(&from_id) {
@@ -1192,11 +1158,11 @@ fn create_or_lookup_adhoc_group(
if !member_ids.contains(&DC_CONTACT_ID_SELF) { if !member_ids.contains(&DC_CONTACT_ID_SELF) {
member_ids.push(DC_CONTACT_ID_SELF); member_ids.push(DC_CONTACT_ID_SELF);
} }
if member_ids.len() < 3 { ensure!(
// too few contacts given member_ids.len() >= 3,
cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked); "Num-contacts={} too low",
return Ok(()); member_ids.len()
} );
let chat_ids = search_chat_ids_by_contact_ids(context, &member_ids)?; let chat_ids = search_chat_ids_by_contact_ids(context, &member_ids)?;
if !chat_ids.is_empty() { if !chat_ids.is_empty() {
@@ -1214,28 +1180,23 @@ fn create_or_lookup_adhoc_group(
); );
if let Ok((id, id_blocked)) = res { if let Ok((id, id_blocked)) = res {
chat_id = id as u32;
chat_id_blocked = id_blocked;
/* success, chat found */ /* success, chat found */
cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked); return Ok((id as u32, id_blocked));
return Ok(());
} }
} }
if 0 == allow_creation { ensure!(
cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked); allow_creation != 0,
return Ok(()); "creating ad-hoc group prevented from caller"
} );
// we do not check if the message is a reply to another group, this may result in // we do not check if the message is a reply to another group, this may result in
// chats with unclear member list. instead we create a new group in the following lines ... // chats with unclear member list. instead we create a new group in the following lines ...
// create a new ad-hoc group // create a new ad-hoc group
// - there is no need to check if this group exists; otherwise we would have caught it above // - there is no need to check if this group exists; otherwise we would have caught it above
let grpid = create_adhoc_grp_id(context, &member_ids); let grpid = create_adhoc_grp_id(context, &member_ids);
if grpid.is_empty() { ensure!(!grpid.is_empty(), "failed to create ad-hoc grpid");
cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked);
return Ok(());
}
// use subject as initial chat name // use subject as initial chat name
let grpname = if let Some(subject) = mime_parser.subject.as_ref().filter(|s| !s.is_empty()) { let grpname = if let Some(subject) = mime_parser.subject.as_ref().filter(|s| !s.is_empty()) {
@@ -1245,22 +1206,20 @@ fn create_or_lookup_adhoc_group(
}; };
// create group record // create group record
chat_id = create_group_record( let chat_id = create_group_record(
context, context,
&grpid, &grpid,
grpname, grpname,
create_blocked, create_blocked,
VerifiedStatus::Unverified, VerifiedStatus::Unverified,
); );
chat_id_blocked = create_blocked;
for &member_id in &member_ids { for &member_id in &member_ids {
chat::add_to_chat_contacts_table(context, chat_id, member_id); chat::add_to_chat_contacts_table(context, chat_id, member_id);
} }
context.call_cb(Event::ChatModified(chat_id)); context.call_cb(Event::ChatModified(chat_id));
cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked); Ok((chat_id, create_blocked))
Ok(())
} }
fn create_group_record( fn create_group_record(