From bb16849eef7339c578577466c5b6cbbd33b86039 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 26 Aug 2019 21:07:53 +0200 Subject: [PATCH 1/6] refactor(location): remove most unsafe usage --- src/dc_array.rs | 12 +- src/dc_location.rs | 388 +++++++++++++++++------------------------- src/dc_mimefactory.rs | 52 +++--- src/dc_mimeparser.rs | 35 ++-- src/dc_receive_imf.rs | 5 +- src/job.rs | 8 +- 6 files changed, 207 insertions(+), 293 deletions(-) diff --git a/src/dc_array.rs b/src/dc_array.rs index a764b6cf6..a69bab3e2 100644 --- a/src/dc_array.rs +++ b/src/dc_array.rs @@ -1,11 +1,11 @@ -use crate::dc_location::dc_location; +use crate::dc_location::Location; use crate::types::*; /* * the structure behind dc_array_t */ #[derive(Clone)] #[allow(non_camel_case_types)] pub enum dc_array_t { - Locations(Vec), + Locations(Vec), Uint(Vec), } @@ -27,7 +27,7 @@ impl dc_array_t { } } - pub fn add_location(&mut self, location: dc_location) { + pub fn add_location(&mut self, location: Location) { if let Self::Locations(array) = self { array.push(location) } else { @@ -42,7 +42,7 @@ impl dc_array_t { } } - pub fn get_location(&self, index: usize) -> &dc_location { + pub fn get_location(&self, index: usize) -> &Location { if let Self::Locations(array) = self { &array[index] } else { @@ -108,8 +108,8 @@ impl From> for dc_array_t { } } -impl From> for dc_array_t { - fn from(array: Vec) -> Self { +impl From> for dc_array_t { + fn from(array: Vec) -> Self { dc_array_t::Locations(array) } } diff --git a/src/dc_location.rs b/src/dc_location.rs index fd1198feb..b218d39f7 100644 --- a/src/dc_location.rs +++ b/src/dc_location.rs @@ -6,69 +6,51 @@ use crate::constants::Event; use crate::constants::*; use crate::context::*; use crate::dc_tools::*; +use crate::error::Error; use crate::job::*; use crate::message::*; use crate::param::*; use crate::sql; use crate::stock::StockMessage; use crate::types::*; -use crate::x::*; // location handling -#[derive(Clone, Default)] -#[allow(non_camel_case_types)] -pub struct dc_location { - pub location_id: uint32_t, - pub latitude: libc::c_double, - pub longitude: libc::c_double, - pub accuracy: libc::c_double, +#[derive(Debug, Clone, Default)] +pub struct Location { + pub location_id: u32, + pub latitude: f64, + pub longitude: f64, + pub accuracy: f64, pub timestamp: i64, - pub contact_id: uint32_t, - pub msg_id: uint32_t, - pub chat_id: uint32_t, + pub contact_id: u32, + pub msg_id: u32, + pub chat_id: u32, pub marker: Option, - pub independent: uint32_t, + pub independent: u32, } -impl dc_location { +impl Location { pub fn new() -> Self { - dc_location { - location_id: 0, - latitude: 0.0, - longitude: 0.0, - accuracy: 0.0, - timestamp: 0, - contact_id: 0, - msg_id: 0, - chat_id: 0, - marker: None, - independent: 0, - } + Default::default() } } -#[derive(Clone)] -#[allow(non_camel_case_types)] -pub struct dc_kml_t { - pub addr: *mut libc::c_char, - pub locations: Option>, - pub tag: libc::c_int, - pub curr: dc_location, +#[derive(Debug, Clone, Default)] +pub struct Kml { + pub addr: Option, + pub locations: Option>, + pub tag: i32, + pub curr: Location, } -impl dc_kml_t { +impl Kml { pub fn new() -> Self { - dc_kml_t { - addr: std::ptr::null_mut(), - locations: None, - tag: 0, - curr: dc_location::new(), - } + Default::default() } } // location streaming -pub unsafe fn dc_send_locations_to_chat(context: &Context, chat_id: uint32_t, seconds: i64) { +pub fn dc_send_locations_to_chat(context: &Context, chat_id: u32, seconds: i64) { let now = time(); let mut msg: Message; let is_sending_locations_before: bool; @@ -94,7 +76,7 @@ pub unsafe fn dc_send_locations_to_chat(context: &Context, chat_id: uint32_t, se msg.text = Some(context.stock_system_msg(StockMessage::MsgLocationEnabled, "", "", 0)); msg.param.set_int(Param::Cmd, 8); - chat::send_msg(context, chat_id, &mut msg).unwrap(); + unsafe { chat::send_msg(context, chat_id, &mut msg).unwrap() }; } else if 0 == seconds && is_sending_locations_before { let stock_str = context.stock_system_msg(StockMessage::MsgLocationDisabled, "", "", 0); @@ -123,7 +105,7 @@ pub unsafe fn dc_send_locations_to_chat(context: &Context, chat_id: uint32_t, se * job to send locations out to all chats that want them ******************************************************************************/ #[allow(non_snake_case)] -unsafe fn schedule_MAYBE_SEND_LOCATIONS(context: &Context, flags: libc::c_int) { +fn schedule_MAYBE_SEND_LOCATIONS(context: &Context, flags: i32) { if 0 != flags & 0x1 || !job_action_exists(context, Action::MaybeSendLocations) { job_add(context, Action::MaybeSendLocations, 0, Params::new(), 60); }; @@ -141,9 +123,9 @@ pub fn dc_is_sending_locations_to_chat(context: &Context, chat_id: u32) -> bool pub fn dc_set_location( context: &Context, - latitude: libc::c_double, - longitude: libc::c_double, - accuracy: libc::c_double, + latitude: f64, + longitude: f64, + accuracy: f64, ) -> libc::c_int { if latitude == 0.0 && longitude == 0.0 { return 1; @@ -174,7 +156,7 @@ pub fn dc_set_location( if continue_streaming { context.call_cb(Event::LOCATION_CHANGED, 1, 0); }; - unsafe { schedule_MAYBE_SEND_LOCATIONS(context, 0) }; + schedule_MAYBE_SEND_LOCATIONS(context, 0); Ok(continue_streaming as libc::c_int) } ).unwrap_or_default() @@ -182,11 +164,11 @@ pub fn dc_set_location( pub fn dc_get_locations( context: &Context, - chat_id: uint32_t, - contact_id: uint32_t, + chat_id: u32, + contact_id: u32, timestamp_from: i64, mut timestamp_to: i64, -) -> Vec { +) -> Vec { if timestamp_to == 0 { timestamp_to = time() + 10; } @@ -217,7 +199,7 @@ pub fn dc_get_locations( None }; - let loc = dc_location { + let loc = Location { location_id: row.get(0)?, latitude: row.get(1)?, longitude: row.get(2)?, @@ -255,22 +237,18 @@ pub fn dc_delete_all_locations(context: &Context) -> bool { true } -pub fn dc_get_location_kml( - context: &Context, - chat_id: uint32_t, - last_added_location_id: *mut uint32_t, -) -> *mut libc::c_char { - let mut success: libc::c_int = 0; +pub fn dc_get_location_kml(context: &Context, chat_id: u32) -> Result<(String, u32), Error> { let now = time(); - let mut location_count: libc::c_int = 0; + let mut location_count = 0; let mut ret = String::new(); + let mut last_added_location_id = 0; let self_addr = context .sql .get_config(context, "configured_addr") .unwrap_or_default(); - if let Ok((locations_send_begin, locations_send_until, locations_last_sent)) = context.sql.query_row( + let (locations_send_begin, locations_send_until, locations_last_sent) = context.sql.query_row( "SELECT locations_send_begin, locations_send_until, locations_last_sent FROM chats WHERE id=?;", params![chat_id as i32], |row| { let send_begin: i64 = row.get(0)?; @@ -278,106 +256,78 @@ pub fn dc_get_location_kml( let last_sent: i64 = row.get(2)?; Ok((send_begin, send_until, last_sent)) - } - ) { - if !(locations_send_begin == 0 || now > locations_send_until) { - ret += &format!( - "\n\n\n", - self_addr, - ); + })?; - context.sql.query_map( - "SELECT id, latitude, longitude, accuracy, timestamp\ - FROM locations WHERE from_id=? \ - AND timestamp>=? \ - AND (timestamp>=? OR timestamp=(SELECT MAX(timestamp) FROM locations WHERE from_id=?)) \ - AND independent=0 \ - GROUP BY timestamp \ - ORDER BY timestamp;", - params![1, locations_send_begin, locations_last_sent, 1], - |row| { - let location_id: i32 = row.get(0)?; - let latitude: f64 = row.get(1)?; - let longitude: f64 = row.get(2)?; - let accuracy: f64 = row.get(3)?; - let timestamp = unsafe { get_kml_timestamp(row.get(4)?) }; + if !(locations_send_begin == 0 || now > locations_send_until) { + ret += &format!( + "\n\n\n", + self_addr, + ); - Ok((location_id, latitude, longitude, accuracy, timestamp)) - }, - |rows| { - for row in rows { - let (location_id, latitude, longitude, accuracy, timestamp) = row?; - ret += &format!( - "{}{},{}\n\x00", - as_str(timestamp), - accuracy, - longitude, - latitude - ); - location_count += 1; - if !last_added_location_id.is_null() { - unsafe { *last_added_location_id = location_id as u32 }; - } - unsafe { free(timestamp as *mut libc::c_void) }; - } - Ok(()) + context.sql.query_map( + "SELECT id, latitude, longitude, accuracy, timestamp\ + FROM locations WHERE from_id=? \ + AND timestamp>=? \ + AND (timestamp>=? OR timestamp=(SELECT MAX(timestamp) FROM locations WHERE from_id=?)) \ + AND independent=0 \ + GROUP BY timestamp \ + ORDER BY timestamp;", + params![1, locations_send_begin, locations_last_sent, 1], + |row| { + let location_id: i32 = row.get(0)?; + let latitude: f64 = row.get(1)?; + let longitude: f64 = row.get(2)?; + let accuracy: f64 = row.get(3)?; + let timestamp = get_kml_timestamp(row.get(4)?); + + Ok((location_id, latitude, longitude, accuracy, timestamp)) + }, + |rows| { + for row in rows { + let (location_id, latitude, longitude, accuracy, timestamp) = row?; + ret += &format!( + "{}{},{}\n\x00", + timestamp, + accuracy, + longitude, + latitude + ); + location_count += 1; + last_added_location_id = location_id as u32; } - ).unwrap(); // TODO: better error handling - } + Ok(()) + } + )?; } - if location_count > 0 { - ret += "\n"; - success = 1; - } + ensure!(location_count > 0, "No locations processed"); + ret += "\n"; - if 0 != success { - unsafe { ret.strdup() } - } else { - std::ptr::null_mut() - } + Ok((ret, last_added_location_id)) } -/******************************************************************************* - * create kml-files - ******************************************************************************/ -unsafe fn get_kml_timestamp(utc: i64) -> *mut libc::c_char { +fn get_kml_timestamp(utc: i64) -> String { // Returns a string formatted as YYYY-MM-DDTHH:MM:SSZ. The trailing `Z` indicates UTC. - let res = chrono::NaiveDateTime::from_timestamp(utc, 0) + chrono::NaiveDateTime::from_timestamp(utc, 0) .format("%Y-%m-%dT%H:%M:%SZ") - .to_string(); - res.strdup() + .to_string() } -pub unsafe fn dc_get_message_kml( - timestamp: i64, - latitude: libc::c_double, - longitude: libc::c_double, -) -> *mut libc::c_char { - let timestamp_str = get_kml_timestamp(timestamp); - let latitude_str = dc_ftoa(latitude); - let longitude_str = dc_ftoa(longitude); - - let ret = dc_mprintf( - b"\n\ +pub fn dc_get_message_kml(timestamp: i64, latitude: f64, longitude: f64) -> String { + format!( + "\n\ \n\ \n\ \ - %s\ - %s,%s\ + {}\ + {:.2},{:.2}\ \n\ \n\ - \x00" as *const u8 as *const libc::c_char, - timestamp_str, - longitude_str, // reverse order! - latitude_str, - ); - - free(latitude_str as *mut libc::c_void); - free(longitude_str as *mut libc::c_void); - free(timestamp_str as *mut libc::c_void); - - ret + ", + get_kml_timestamp(timestamp), + longitude, + latitude, + ) } pub fn dc_set_kml_sent_timestamp(context: &Context, chat_id: u32, timestamp: i64) -> bool { @@ -400,12 +350,12 @@ pub fn dc_set_msg_location_id(context: &Context, msg_id: u32, location_id: u32) .is_ok() } -pub unsafe fn dc_save_locations( +pub fn dc_save_locations( context: &Context, chat_id: u32, contact_id: u32, - locations_opt: &Option>, - independent: libc::c_int, + locations_opt: &Option>, + independent: i32, ) -> u32 { if chat_id <= 9 || locations_opt.is_none() { return 0; @@ -458,59 +408,47 @@ pub unsafe fn dc_save_locations( .unwrap_or_default() } -pub unsafe fn dc_kml_parse( - context: &Context, - content: *const libc::c_char, - content_bytes: size_t, -) -> dc_kml_t { - let mut kml = dc_kml_t::new(); +pub fn dc_kml_parse(context: &Context, content: impl AsRef) -> Result { + ensure!( + content.as_ref().len() <= (1 * 1024 * 1024), + "A kml-files with {} bytes is larger than reasonably expected.", + content.as_ref().len() + ); - if content_bytes > (1 * 1024 * 1024) { - warn!( - context, - 0, "A kml-files with {} bytes is larger than reasonably expected.", content_bytes, - ); - return kml; - } + let mut reader = quick_xml::Reader::from_str(content.as_ref()); + reader.trim_text(true); - let content_null = dc_null_terminate(content, content_bytes as libc::c_int); - if !content_null.is_null() { - let mut reader = quick_xml::Reader::from_str(as_str(content_null)); - reader.trim_text(true); + let mut kml = Kml::new(); + kml.locations = Some(Vec::with_capacity(100)); - kml.locations = Some(Vec::with_capacity(100)); + let mut buf = Vec::new(); - let mut buf = Vec::new(); - - loop { - match reader.read_event(&mut buf) { - Ok(quick_xml::events::Event::Start(ref e)) => kml_starttag_cb(e, &mut kml, &reader), - Ok(quick_xml::events::Event::End(ref e)) => kml_endtag_cb(e, &mut kml), - Ok(quick_xml::events::Event::Text(ref e)) => kml_text_cb(e, &mut kml, &reader), - Err(e) => { - error!( - context, - 0, - "Location parsing: Error at position {}: {:?}", - reader.buffer_position(), - e - ); - } - Ok(quick_xml::events::Event::Eof) => break, - _ => (), + loop { + match reader.read_event(&mut buf) { + Ok(quick_xml::events::Event::Start(ref e)) => kml_starttag_cb(e, &mut kml, &reader), + Ok(quick_xml::events::Event::End(ref e)) => kml_endtag_cb(e, &mut kml), + Ok(quick_xml::events::Event::Text(ref e)) => kml_text_cb(e, &mut kml, &reader), + Err(e) => { + error!( + context, + 0, + "Location parsing: Error at position {}: {:?}", + reader.buffer_position(), + e + ); } - buf.clear(); + Ok(quick_xml::events::Event::Eof) => break, + _ => (), } + buf.clear(); } - free(content_null.cast()); - - kml + Ok(kml) } fn kml_text_cb( event: &BytesText, - kml: &mut dc_kml_t, + kml: &mut Kml, reader: &quick_xml::Reader, ) { if 0 != kml.tag & (0x4 | 0x10) { @@ -546,7 +484,7 @@ fn kml_text_cb( } } -fn kml_endtag_cb(event: &BytesEnd, kml: &mut dc_kml_t) { +fn kml_endtag_cb(event: &BytesEnd, kml: &mut Kml) { let tag = String::from_utf8_lossy(event.name()).trim().to_lowercase(); if tag == "placemark" { @@ -556,7 +494,7 @@ fn kml_endtag_cb(event: &BytesEnd, kml: &mut dc_kml_t) { && 0. != kml.curr.longitude { if let Some(ref mut locations) = kml.locations { - locations.push(std::mem::replace(&mut kml.curr, dc_location::new())); + locations.push(std::mem::replace(&mut kml.curr, Location::new())); } } kml.tag = 0 @@ -565,7 +503,7 @@ fn kml_endtag_cb(event: &BytesEnd, kml: &mut dc_kml_t) { fn kml_starttag_cb( event: &BytesStart, - kml: &mut dc_kml_t, + kml: &mut Kml, reader: &quick_xml::Reader, ) { let tag = String::from_utf8_lossy(event.name()).trim().to_lowercase(); @@ -575,19 +513,14 @@ fn kml_starttag_cb( .map(|a| String::from_utf8_lossy(a.key).trim().to_lowercase() == "addr") .unwrap_or_default() }) { - kml.addr = unsafe { - addr.unwrap() - .unescape_and_decode_value(reader) - .unwrap_or_default() - .strdup() - }; + kml.addr = addr.unwrap().unescape_and_decode_value(reader).ok(); } } else if tag == "placemark" { kml.tag = 0x1; kml.curr.timestamp = 0; - kml.curr.latitude = 0 as libc::c_double; - kml.curr.longitude = 0.0f64; - kml.curr.accuracy = 0.0f64 + kml.curr.latitude = 0.0; + kml.curr.longitude = 0.0; + kml.curr.accuracy = 0.0 } else if tag == "timestamp" && 0 != kml.tag & 0x1 { kml.tag = 0x1 | 0x2 } else if tag == "when" && 0 != kml.tag & 0x2 { @@ -611,12 +544,8 @@ fn kml_starttag_cb( } } -pub unsafe fn dc_kml_unref(kml: &mut dc_kml_t) { - free(kml.addr as *mut libc::c_void); -} - #[allow(non_snake_case)] -pub unsafe fn dc_job_do_DC_JOB_MAYBE_SEND_LOCATIONS(context: &Context, _job: &Job) { +pub fn dc_job_do_DC_JOB_MAYBE_SEND_LOCATIONS(context: &Context, _job: &Job) { let now = time(); let mut continue_streaming: libc::c_int = 1; info!( @@ -682,7 +611,7 @@ pub unsafe fn dc_job_do_DC_JOB_MAYBE_SEND_LOCATIONS(context: &Context, _job: &Jo msg.hidden = true; msg.param.set_int(Param::Cmd, 9); // TODO: handle cleanup on error - chat::send_msg(context, chat_id as u32, &mut msg).unwrap(); + unsafe { chat::send_msg(context, chat_id as u32, &mut msg).unwrap() }; } Ok(()) }, @@ -697,12 +626,12 @@ pub unsafe fn dc_job_do_DC_JOB_MAYBE_SEND_LOCATIONS(context: &Context, _job: &Jo } #[allow(non_snake_case)] -pub unsafe fn dc_job_do_DC_JOB_MAYBE_SEND_LOC_ENDED(context: &Context, job: &mut Job) { +pub fn dc_job_do_DC_JOB_MAYBE_SEND_LOC_ENDED(context: &Context, job: &mut Job) { // this function is called when location-streaming _might_ have ended for a chat. // the function checks, if location-streaming is really ended; // if so, a device-message is added if not yet done. - let chat_id = (*job).foreign_id; + let chat_id = job.foreign_id; if let Ok((send_begin, send_until)) = context.sql.query_row( "SELECT locations_send_begin, locations_send_until FROM chats WHERE id=?", @@ -739,38 +668,33 @@ mod tests { #[test] fn test_dc_kml_parse() { - unsafe { - let context = dummy_context(); + let context = dummy_context(); - let xml = - b"\n\n\n2019-03-06T21:09:57Z9.423110,53.790302\n\n \n\t2018-12-13T22:11:12Z\t 19.423110 \t , \n 63.790302\n \n\n\x00" - as *const u8 as *const libc::c_char; + let xml = + "\n\n\n2019-03-06T21:09:57Z9.423110,53.790302\n\n \n\t2018-12-13T22:11:12Z\t 19.423110 \t , \n 63.790302\n \n\n"; - let mut kml = dc_kml_parse(&context.ctx, xml, strlen(xml)); + let kml = dc_kml_parse(&context.ctx, &xml).expect("parsing failed"); - assert!(!kml.addr.is_null()); - assert_eq!(as_str(kml.addr as *const libc::c_char), "user@example.org",); + assert!(kml.addr.is_some()); + assert_eq!(kml.addr.as_ref().unwrap(), "user@example.org",); - let locations_ref = &kml.locations.as_ref().unwrap(); - assert_eq!(locations_ref.len(), 2); + let locations_ref = &kml.locations.as_ref().unwrap(); + assert_eq!(locations_ref.len(), 2); - assert!(locations_ref[0].latitude > 53.6f64); - assert!(locations_ref[0].latitude < 53.8f64); - assert!(locations_ref[0].longitude > 9.3f64); - assert!(locations_ref[0].longitude < 9.5f64); - assert!(locations_ref[0].accuracy > 31.9f64); - assert!(locations_ref[0].accuracy < 32.1f64); - assert_eq!(locations_ref[0].timestamp, 1551906597); + assert!(locations_ref[0].latitude > 53.6f64); + assert!(locations_ref[0].latitude < 53.8f64); + assert!(locations_ref[0].longitude > 9.3f64); + assert!(locations_ref[0].longitude < 9.5f64); + assert!(locations_ref[0].accuracy > 31.9f64); + assert!(locations_ref[0].accuracy < 32.1f64); + assert_eq!(locations_ref[0].timestamp, 1551906597); - assert!(locations_ref[1].latitude > 63.6f64); - assert!(locations_ref[1].latitude < 63.8f64); - assert!(locations_ref[1].longitude > 19.3f64); - assert!(locations_ref[1].longitude < 19.5f64); - assert!(locations_ref[1].accuracy > 2.4f64); - assert!(locations_ref[1].accuracy < 2.6f64); - assert_eq!(locations_ref[1].timestamp, 1544739072); - - dc_kml_unref(&mut kml); - } + assert!(locations_ref[1].latitude > 63.6f64); + assert!(locations_ref[1].latitude < 63.8f64); + assert!(locations_ref[1].longitude > 19.3f64); + assert!(locations_ref[1].longitude < 19.5f64); + assert!(locations_ref[1].accuracy > 2.4f64); + assert!(locations_ref[1].accuracy < 2.6f64); + assert_eq!(locations_ref[1].timestamp, 1544739072); } } diff --git a/src/dc_mimefactory.rs b/src/dc_mimefactory.rs index b844040b2..4919f9963 100644 --- a/src/dc_mimefactory.rs +++ b/src/dc_mimefactory.rs @@ -885,43 +885,39 @@ pub unsafe fn dc_mimefactory_render(factory: &mut dc_mimefactory_t) -> libc::c_i .unwrap_or_default(); let kml_file = dc_get_message_kml(factory.msg.timestamp_sort, latitude, longitude); - if !kml_file.is_null() { + let content_type = mailmime_content_new_with_str( + b"application/vnd.google-earth.kml+xml\x00" as *const u8 + as *const libc::c_char, + ); + let mime_fields = mailmime_fields_new_filename( + MAILMIME_DISPOSITION_TYPE_ATTACHMENT as libc::c_int, + dc_strdup(b"message.kml\x00" as *const u8 as *const libc::c_char), + MAILMIME_MECHANISM_8BIT as libc::c_int, + ); + let kml_mime_part = mailmime_new_empty(content_type, mime_fields); + mailmime_set_body_text(kml_mime_part, kml_file.strdup(), kml_file.len()); + mailmime_smart_add_part(message, kml_mime_part); + } + + if dc_is_sending_locations_to_chat(factory.msg.context, factory.msg.chat_id) { + if let Ok((kml_file, last_added_location_id)) = + dc_get_location_kml(factory.msg.context, factory.msg.chat_id) + { let content_type = mailmime_content_new_with_str( b"application/vnd.google-earth.kml+xml\x00" as *const u8 as *const libc::c_char, ); let mime_fields = mailmime_fields_new_filename( - MAILMIME_DISPOSITION_TYPE_ATTACHMENT as libc::c_int, - dc_strdup(b"message.kml\x00" as *const u8 as *const libc::c_char), - MAILMIME_MECHANISM_8BIT as libc::c_int, - ); - let kml_mime_part = mailmime_new_empty(content_type, mime_fields); - mailmime_set_body_text(kml_mime_part, kml_file, strlen(kml_file)); - - mailmime_smart_add_part(message, kml_mime_part); - } - } - - if dc_is_sending_locations_to_chat(factory.msg.context, factory.msg.chat_id) { - let mut last_added_location_id: uint32_t = 0 as uint32_t; - let kml_file: *mut libc::c_char = dc_get_location_kml( - factory.msg.context, - factory.msg.chat_id, - &mut last_added_location_id, - ); - if !kml_file.is_null() { - let content_type: *mut mailmime_content = mailmime_content_new_with_str( - b"application/vnd.google-earth.kml+xml\x00" as *const u8 - as *const libc::c_char, - ); - let mime_fields: *mut mailmime_fields = mailmime_fields_new_filename( MAILMIME_DISPOSITION_TYPE_ATTACHMENT as libc::c_int, dc_strdup(b"location.kml\x00" as *const u8 as *const libc::c_char), MAILMIME_MECHANISM_8BIT as libc::c_int, ); - let kml_mime_part: *mut mailmime = - mailmime_new_empty(content_type, mime_fields); - mailmime_set_body_text(kml_mime_part, kml_file, strlen(kml_file)); + let kml_mime_part = mailmime_new_empty(content_type, mime_fields); + mailmime_set_body_text( + kml_mime_part, + kml_file.strdup(), + kml_file.len(), + ); mailmime_smart_add_part(message, kml_mime_part); if !factory.msg.param.exists(Param::SetLatitude) { // otherwise, the independent location is already filed diff --git a/src/dc_mimeparser.rs b/src/dc_mimeparser.rs index 118c61258..7dbb875fa 100644 --- a/src/dc_mimeparser.rs +++ b/src/dc_mimeparser.rs @@ -57,8 +57,8 @@ pub struct dc_mimeparser_t<'a> { pub context: &'a Context, pub reports: Vec<*mut mailmime>, pub is_system_message: libc::c_int, - pub location_kml: Option, - pub message_kml: Option, + pub location_kml: Option, + pub message_kml: Option, } // deprecated: flag to switch generation of compound messages on and off. @@ -112,14 +112,7 @@ unsafe fn dc_mimeparser_empty(mimeparser: &mut dc_mimeparser_t) { mimeparser.decrypting_failed = 0i32; dc_e2ee_thanks(&mut mimeparser.e2ee_helper); - if let Some(location_kml) = mimeparser.location_kml.as_mut() { - dc_kml_unref(location_kml); - } mimeparser.location_kml = None; - - if let Some(message_kml) = mimeparser.message_kml.as_mut() { - dc_kml_unref(message_kml); - } mimeparser.message_kml = None; } @@ -1300,11 +1293,13 @@ unsafe fn dc_mimeparser_add_single_part_if_known( 4, ) == 0i32 { - mimeparser.location_kml = Some(dc_kml_parse( - mimeparser.context, - decoded_data, - decoded_data_bytes, - )); + if !decoded_data.is_null() && decoded_data_bytes > 0 { + let d = + dc_null_terminate(decoded_data, decoded_data_bytes as i32); + mimeparser.location_kml = + dc_kml_parse(mimeparser.context, as_str(d)).ok(); + free(d.cast()); + } } else if strncmp( desired_filename, b"message\x00" as *const u8 as *const libc::c_char, @@ -1318,11 +1313,13 @@ unsafe fn dc_mimeparser_add_single_part_if_known( 4, ) == 0i32 { - mimeparser.message_kml = Some(dc_kml_parse( - mimeparser.context, - decoded_data, - decoded_data_bytes, - )); + if !decoded_data.is_null() && decoded_data_bytes > 0 { + let d = + dc_null_terminate(decoded_data, decoded_data_bytes as i32); + mimeparser.message_kml = + dc_kml_parse(mimeparser.context, as_str(d)).ok(); + free(d.cast()); + } } else { dc_replace_bad_utf8_chars(desired_filename); do_add_single_file_part( diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index 1b834de02..5fbf179a2 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -953,11 +953,10 @@ unsafe fn save_locations( } if !mime_parser.location_kml.is_none() && chat_id > DC_CHAT_ID_LAST_SPECIAL as libc::c_uint { - if !mime_parser.location_kml.as_ref().unwrap().addr.is_null() { + if let Some(ref addr) = mime_parser.location_kml.as_ref().unwrap().addr { if let Ok(contact) = Contact::get_by_id(context, from_id) { if !contact.get_addr().is_empty() - && contact.get_addr().to_lowercase() - == as_str(mime_parser.location_kml.as_ref().unwrap().addr).to_lowercase() + && contact.get_addr().to_lowercase() == addr.to_lowercase() { let newest_location_id = dc_save_locations( context, diff --git a/src/job.rs b/src/job.rs index 0bb313af1..e98dae1ab 100644 --- a/src/job.rs +++ b/src/job.rs @@ -875,12 +875,10 @@ fn job_perform(context: &Context, thread: Thread, probe_network: bool) { Action::SendMdn => job.do_DC_JOB_SEND(context), Action::ConfigureImap => unsafe { dc_job_do_DC_JOB_CONFIGURE_IMAP(context, &job) }, Action::ImexImap => unsafe { dc_job_do_DC_JOB_IMEX_IMAP(context, &job) }, - Action::MaybeSendLocations => unsafe { - dc_job_do_DC_JOB_MAYBE_SEND_LOCATIONS(context, &job) - }, - Action::MaybeSendLocationsEnded => unsafe { + Action::MaybeSendLocations => dc_job_do_DC_JOB_MAYBE_SEND_LOCATIONS(context, &job), + Action::MaybeSendLocationsEnded => { dc_job_do_DC_JOB_MAYBE_SEND_LOC_ENDED(context, &mut job) - }, + } Action::Housekeeping => sql::housekeeping(context), Action::SendMdnOld => {} Action::SendMsgToSmtpOld => {} From 23d49560bffbc15eaec481c1f6147143f3edecbf Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 26 Aug 2019 21:09:40 +0200 Subject: [PATCH 2/6] refactor(location): rename module dc_location -> location --- deltachat-ffi/src/lib.rs | 10 +++++----- examples/repl/cmdline.rs | 2 +- src/dc_array.rs | 2 +- src/dc_mimefactory.rs | 2 +- src/dc_mimeparser.rs | 2 +- src/dc_receive_imf.rs | 2 +- src/job.rs | 2 +- src/lib.rs | 2 +- src/{dc_location.rs => location.rs} | 0 9 files changed, 12 insertions(+), 12 deletions(-) rename src/{dc_location.rs => location.rs} (100%) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index c919d660d..2d654ba03 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -1043,7 +1043,7 @@ pub unsafe extern "C" fn dc_send_locations_to_chat( assert!(!context.is_null()); let context = &*context; - dc_location::dc_send_locations_to_chat(context, chat_id, seconds as i64) + location::dc_send_locations_to_chat(context, chat_id, seconds as i64) } #[no_mangle] @@ -1054,7 +1054,7 @@ pub unsafe extern "C" fn dc_is_sending_locations_to_chat( assert!(!context.is_null()); let context = &*context; - dc_location::dc_is_sending_locations_to_chat(context, chat_id) as libc::c_int + location::dc_is_sending_locations_to_chat(context, chat_id) as libc::c_int } #[no_mangle] @@ -1067,7 +1067,7 @@ pub unsafe extern "C" fn dc_set_location( assert!(!context.is_null()); let context = &*context; - dc_location::dc_set_location(context, latitude, longitude, accuracy) + location::dc_set_location(context, latitude, longitude, accuracy) } #[no_mangle] @@ -1081,7 +1081,7 @@ pub unsafe extern "C" fn dc_get_locations( assert!(!context.is_null()); let context = &*context; - let res = dc_location::dc_get_locations( + let res = location::dc_get_locations( context, chat_id, contact_id, @@ -1096,7 +1096,7 @@ pub unsafe extern "C" fn dc_delete_all_locations(context: *mut dc_context_t) { assert!(!context.is_null()); let context = &*context; - dc_location::dc_delete_all_locations(context); + location::dc_delete_all_locations(context); } // dc_array_t diff --git a/examples/repl/cmdline.rs b/examples/repl/cmdline.rs index cbb06289b..3cded24ff 100644 --- a/examples/repl/cmdline.rs +++ b/examples/repl/cmdline.rs @@ -9,11 +9,11 @@ use deltachat::constants::*; use deltachat::contact::*; use deltachat::context::*; use deltachat::dc_imex::*; -use deltachat::dc_location::*; use deltachat::dc_receive_imf::*; use deltachat::dc_tools::*; use deltachat::error::Error; use deltachat::job::*; +use deltachat::location::*; use deltachat::lot::LotState; use deltachat::message::*; use deltachat::peerstate::*; diff --git a/src/dc_array.rs b/src/dc_array.rs index a69bab3e2..c3e2dcbe0 100644 --- a/src/dc_array.rs +++ b/src/dc_array.rs @@ -1,4 +1,4 @@ -use crate::dc_location::Location; +use crate::location::Location; use crate::types::*; /* * the structure behind dc_array_t */ diff --git a/src/dc_mimefactory.rs b/src/dc_mimefactory.rs index 4919f9963..fc146214c 100644 --- a/src/dc_mimefactory.rs +++ b/src/dc_mimefactory.rs @@ -16,10 +16,10 @@ use crate::constants::*; use crate::contact::*; use crate::context::{dc_get_version_str, Context}; use crate::dc_e2ee::*; -use crate::dc_location::*; use crate::dc_strencode::*; use crate::dc_tools::*; use crate::error::Error; +use crate::location::*; use crate::message::*; use crate::param::*; use crate::stock::StockMessage; diff --git a/src/dc_mimeparser.rs b/src/dc_mimeparser.rs index 7dbb875fa..8109f8673 100644 --- a/src/dc_mimeparser.rs +++ b/src/dc_mimeparser.rs @@ -14,10 +14,10 @@ use mmime::other::*; use crate::contact::*; use crate::context::Context; use crate::dc_e2ee::*; -use crate::dc_location::*; use crate::dc_simplify::*; use crate::dc_strencode::*; use crate::dc_tools::*; +use crate::location::*; use crate::param::*; use crate::stock::StockMessage; use crate::types::*; diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index 5fbf179a2..43d3add6a 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -14,7 +14,6 @@ use crate::chat::{self, Chat}; use crate::constants::*; use crate::contact::*; use crate::context::Context; -use crate::dc_location::*; use crate::dc_mimeparser::*; use crate::dc_move::*; use crate::dc_securejoin::*; @@ -22,6 +21,7 @@ use crate::dc_strencode::*; use crate::dc_tools::*; use crate::error::Result; use crate::job::*; +use crate::location::*; use crate::message::*; use crate::param::*; use crate::peerstate::*; diff --git a/src/job.rs b/src/job.rs index e98dae1ab..e7286c711 100644 --- a/src/job.rs +++ b/src/job.rs @@ -10,11 +10,11 @@ use crate::configure::*; use crate::constants::*; use crate::context::Context; use crate::dc_imex::*; -use crate::dc_location::*; use crate::dc_loginparam::*; use crate::dc_mimefactory::*; use crate::dc_tools::*; use crate::imap::*; +use crate::location::*; use crate::message::*; use crate::param::*; use crate::sql; diff --git a/src/lib.rs b/src/lib.rs index ee0766149..cfb4ca51c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,6 +35,7 @@ pub mod job; mod job_thread; pub mod key; pub mod keyring; +pub mod location; pub mod lot; pub mod message; pub mod oauth2; @@ -52,7 +53,6 @@ pub mod dc_array; mod dc_dehtml; mod dc_e2ee; pub mod dc_imex; -pub mod dc_location; mod dc_loginparam; mod dc_mimefactory; pub mod dc_mimeparser; diff --git a/src/dc_location.rs b/src/location.rs similarity index 100% rename from src/dc_location.rs rename to src/location.rs From d25d839d6a717f1e9f87bf0680ca7194f939b6dc Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 26 Aug 2019 21:29:40 +0200 Subject: [PATCH 3/6] refactor(location): more rusty api --- deltachat-ffi/src/lib.rs | 10 +- examples/repl/cmdline.rs | 17 +- src/dc_mimefactory.rs | 16 +- src/dc_mimeparser.rs | 10 +- src/dc_receive_imf.rs | 49 ++--- src/job.rs | 20 +- src/location.rs | 404 +++++++++++++++++++-------------------- 7 files changed, 266 insertions(+), 260 deletions(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 2d654ba03..ca255ae2b 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -1043,7 +1043,7 @@ pub unsafe extern "C" fn dc_send_locations_to_chat( assert!(!context.is_null()); let context = &*context; - location::dc_send_locations_to_chat(context, chat_id, seconds as i64) + location::send_locations_to_chat(context, chat_id, seconds as i64) } #[no_mangle] @@ -1054,7 +1054,7 @@ pub unsafe extern "C" fn dc_is_sending_locations_to_chat( assert!(!context.is_null()); let context = &*context; - location::dc_is_sending_locations_to_chat(context, chat_id) as libc::c_int + location::is_sending_locations_to_chat(context, chat_id) as libc::c_int } #[no_mangle] @@ -1067,7 +1067,7 @@ pub unsafe extern "C" fn dc_set_location( assert!(!context.is_null()); let context = &*context; - location::dc_set_location(context, latitude, longitude, accuracy) + location::set(context, latitude, longitude, accuracy) } #[no_mangle] @@ -1081,7 +1081,7 @@ pub unsafe extern "C" fn dc_get_locations( assert!(!context.is_null()); let context = &*context; - let res = location::dc_get_locations( + let res = location::get_range( context, chat_id, contact_id, @@ -1096,7 +1096,7 @@ pub unsafe extern "C" fn dc_delete_all_locations(context: *mut dc_context_t) { assert!(!context.is_null()); let context = &*context; - location::dc_delete_all_locations(context); + location::delete_all(context); } // dc_array_t diff --git a/examples/repl/cmdline.rs b/examples/repl/cmdline.rs index 3cded24ff..170870881 100644 --- a/examples/repl/cmdline.rs +++ b/examples/repl/cmdline.rs @@ -13,7 +13,7 @@ use deltachat::dc_receive_imf::*; use deltachat::dc_tools::*; use deltachat::error::Error; use deltachat::job::*; -use deltachat::location::*; +use deltachat::location; use deltachat::lot::LotState; use deltachat::message::*; use deltachat::peerstate::*; @@ -652,7 +652,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E ); } } - if dc_is_sending_locations_to_chat(context, 0 as uint32_t) { + if location::is_sending_locations_to_chat(context, 0 as uint32_t) { info!(context, 0, "Location streaming enabled."); } println!("{} chats", cnt); @@ -778,14 +778,17 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E println!( "{} contacts\nLocation streaming: {}", contacts.len(), - dc_is_sending_locations_to_chat(context, sel_chat.as_ref().unwrap().get_id()), + location::is_sending_locations_to_chat( + context, + sel_chat.as_ref().unwrap().get_id() + ), ); } "getlocations" => { ensure!(sel_chat.is_some(), "No chat selected."); let contact_id = arg1.parse().unwrap_or_default(); - let locations = dc_get_locations( + let locations = location::get_range( context, sel_chat.as_ref().unwrap().get_id(), contact_id, @@ -819,7 +822,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E ensure!(!arg1.is_empty(), "No timeout given."); let seconds = arg1.parse()?; - dc_send_locations_to_chat(context, sel_chat.as_ref().unwrap().get_id(), seconds); + location::send_locations_to_chat(context, sel_chat.as_ref().unwrap().get_id(), seconds); println!( "Locations will be sent to Chat#{} for {} seconds. Use 'setlocation ' to play around.", sel_chat.as_ref().unwrap().get_id(), @@ -834,7 +837,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E let latitude = arg1.parse()?; let longitude = arg2.parse()?; - let continue_streaming = dc_set_location(context, latitude, longitude, 0.); + let continue_streaming = location::set(context, latitude, longitude, 0.); if 0 != continue_streaming { println!("Success, streaming should be continued."); } else { @@ -842,7 +845,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E } } "dellocations" => { - dc_delete_all_locations(context); + location::delete_all(context)?; } "send" => { ensure!(sel_chat.is_some(), "No chat selected."); diff --git a/src/dc_mimefactory.rs b/src/dc_mimefactory.rs index fc146214c..0b5622548 100644 --- a/src/dc_mimefactory.rs +++ b/src/dc_mimefactory.rs @@ -19,7 +19,7 @@ use crate::dc_e2ee::*; use crate::dc_strencode::*; use crate::dc_tools::*; use crate::error::Error; -use crate::location::*; +use crate::location; use crate::message::*; use crate::param::*; use crate::stock::StockMessage; @@ -883,8 +883,11 @@ pub unsafe fn dc_mimefactory_render(factory: &mut dc_mimefactory_t) -> libc::c_i .param .get_float(Param::SetLongitude) .unwrap_or_default(); - let kml_file = - dc_get_message_kml(factory.msg.timestamp_sort, latitude, longitude); + let kml_file = location::get_message_kml( + factory.msg.timestamp_sort, + latitude, + longitude, + ); let content_type = mailmime_content_new_with_str( b"application/vnd.google-earth.kml+xml\x00" as *const u8 as *const libc::c_char, @@ -899,9 +902,12 @@ pub unsafe fn dc_mimefactory_render(factory: &mut dc_mimefactory_t) -> libc::c_i mailmime_smart_add_part(message, kml_mime_part); } - if dc_is_sending_locations_to_chat(factory.msg.context, factory.msg.chat_id) { + if location::is_sending_locations_to_chat( + factory.msg.context, + factory.msg.chat_id, + ) { if let Ok((kml_file, last_added_location_id)) = - dc_get_location_kml(factory.msg.context, factory.msg.chat_id) + location::get_kml(factory.msg.context, factory.msg.chat_id) { let content_type = mailmime_content_new_with_str( b"application/vnd.google-earth.kml+xml\x00" as *const u8 diff --git a/src/dc_mimeparser.rs b/src/dc_mimeparser.rs index 8109f8673..8a208a5b3 100644 --- a/src/dc_mimeparser.rs +++ b/src/dc_mimeparser.rs @@ -17,7 +17,7 @@ use crate::dc_e2ee::*; use crate::dc_simplify::*; use crate::dc_strencode::*; use crate::dc_tools::*; -use crate::location::*; +use crate::location; use crate::param::*; use crate::stock::StockMessage; use crate::types::*; @@ -57,8 +57,8 @@ pub struct dc_mimeparser_t<'a> { pub context: &'a Context, pub reports: Vec<*mut mailmime>, pub is_system_message: libc::c_int, - pub location_kml: Option, - pub message_kml: Option, + pub location_kml: Option, + pub message_kml: Option, } // deprecated: flag to switch generation of compound messages on and off. @@ -1297,7 +1297,7 @@ unsafe fn dc_mimeparser_add_single_part_if_known( let d = dc_null_terminate(decoded_data, decoded_data_bytes as i32); mimeparser.location_kml = - dc_kml_parse(mimeparser.context, as_str(d)).ok(); + location::Kml::parse(mimeparser.context, as_str(d)).ok(); free(d.cast()); } } else if strncmp( @@ -1317,7 +1317,7 @@ unsafe fn dc_mimeparser_add_single_part_if_known( let d = dc_null_terminate(decoded_data, decoded_data_bytes as i32); mimeparser.message_kml = - dc_kml_parse(mimeparser.context, as_str(d)).ok(); + location::Kml::parse(mimeparser.context, as_str(d)).ok(); free(d.cast()); } } else { diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index 43d3add6a..f3764d45b 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -21,7 +21,7 @@ use crate::dc_strencode::*; use crate::dc_tools::*; use crate::error::Result; use crate::job::*; -use crate::location::*; +use crate::location; use crate::message::*; use crate::param::*; use crate::peerstate::*; @@ -938,17 +938,16 @@ unsafe fn save_locations( let mut send_event = false; if !mime_parser.message_kml.is_none() && chat_id > DC_CHAT_ID_LAST_SPECIAL as libc::c_uint { - let newest_location_id: uint32_t = dc_save_locations( - context, - chat_id, - from_id, - &mime_parser.message_kml.as_ref().unwrap().locations, - 1, - ); - if 0 != newest_location_id && 0 == hidden { - dc_set_msg_location_id(context, insert_msg_id, newest_location_id); - location_id_written = true; - send_event = true; + if let Some(ref locations) = mime_parser.message_kml.as_ref().unwrap().locations { + let newest_location_id = + location::save(context, chat_id, from_id, locations, 1).unwrap_or_default(); + if 0 != newest_location_id && 0 == hidden { + if location::set_msg_location_id(context, insert_msg_id, newest_location_id).is_ok() + { + location_id_written = true; + send_event = true; + } + } } } @@ -958,17 +957,23 @@ unsafe fn save_locations( if !contact.get_addr().is_empty() && contact.get_addr().to_lowercase() == addr.to_lowercase() { - let newest_location_id = dc_save_locations( - context, - chat_id, - from_id, - &mime_parser.location_kml.as_ref().unwrap().locations, - 0, - ); - if newest_location_id != 0 && hidden == 0 && !location_id_written { - dc_set_msg_location_id(context, insert_msg_id, newest_location_id); + if let Some(ref locations) = + mime_parser.location_kml.as_ref().unwrap().locations + { + let newest_location_id = + location::save(context, chat_id, from_id, locations, 0) + .unwrap_or_default(); + if newest_location_id != 0 && hidden == 0 && !location_id_written { + if let Err(err) = location::set_msg_location_id( + context, + insert_msg_id, + newest_location_id, + ) { + error!(context, 0, "Failed to set msg_location_id: {:?}", err); + } + } + send_event = true; } - send_event = true; } } } diff --git a/src/job.rs b/src/job.rs index e7286c711..0b42c5945 100644 --- a/src/job.rs +++ b/src/job.rs @@ -14,7 +14,7 @@ use crate::dc_loginparam::*; use crate::dc_mimefactory::*; use crate::dc_tools::*; use crate::imap::*; -use crate::location::*; +use crate::location; use crate::message::*; use crate::param::*; use crate::sql; @@ -739,13 +739,19 @@ pub unsafe fn job_send_msg(context: &Context, msg_id: uint32_t) -> libc::c_int { chat::set_gossiped_timestamp(context, mimefactory.msg.chat_id, time()); } if 0 != mimefactory.out_last_added_location_id { - dc_set_kml_sent_timestamp(context, mimefactory.msg.chat_id, time()); + if let Err(err) = + location::set_kml_sent_timestamp(context, mimefactory.msg.chat_id, time()) + { + error!(context, 0, "Failed to set kml sent_timestamp: {:?}", err); + } if !mimefactory.msg.hidden { - dc_set_msg_location_id( + if let Err(err) = location::set_msg_location_id( context, mimefactory.msg.id, mimefactory.out_last_added_location_id, - ); + ) { + error!(context, 0, "Failed to set msg_location_id: {:?}", err); + } } } if 0 != mimefactory.out_encrypted @@ -875,9 +881,11 @@ fn job_perform(context: &Context, thread: Thread, probe_network: bool) { Action::SendMdn => job.do_DC_JOB_SEND(context), Action::ConfigureImap => unsafe { dc_job_do_DC_JOB_CONFIGURE_IMAP(context, &job) }, Action::ImexImap => unsafe { dc_job_do_DC_JOB_IMEX_IMAP(context, &job) }, - Action::MaybeSendLocations => dc_job_do_DC_JOB_MAYBE_SEND_LOCATIONS(context, &job), + Action::MaybeSendLocations => { + location::job_do_DC_JOB_MAYBE_SEND_LOCATIONS(context, &job) + } Action::MaybeSendLocationsEnded => { - dc_job_do_DC_JOB_MAYBE_SEND_LOC_ENDED(context, &mut job) + location::job_do_DC_JOB_MAYBE_SEND_LOC_ENDED(context, &mut job) } Action::Housekeeping => sql::housekeeping(context), Action::SendMdnOld => {} diff --git a/src/location.rs b/src/location.rs index b218d39f7..abfd4101f 100644 --- a/src/location.rs +++ b/src/location.rs @@ -47,15 +47,147 @@ impl Kml { pub fn new() -> Self { Default::default() } + + pub fn parse(context: &Context, content: impl AsRef) -> Result { + ensure!( + content.as_ref().len() <= (1 * 1024 * 1024), + "A kml-files with {} bytes is larger than reasonably expected.", + content.as_ref().len() + ); + + let mut reader = quick_xml::Reader::from_str(content.as_ref()); + reader.trim_text(true); + + let mut kml = Kml::new(); + kml.locations = Some(Vec::with_capacity(100)); + + let mut buf = Vec::new(); + + loop { + match reader.read_event(&mut buf) { + Ok(quick_xml::events::Event::Start(ref e)) => kml.starttag_cb(e, &reader), + Ok(quick_xml::events::Event::End(ref e)) => kml.endtag_cb(e), + Ok(quick_xml::events::Event::Text(ref e)) => kml.text_cb(e, &reader), + Err(e) => { + error!( + context, + 0, + "Location parsing: Error at position {}: {:?}", + reader.buffer_position(), + e + ); + } + Ok(quick_xml::events::Event::Eof) => break, + _ => (), + } + buf.clear(); + } + + Ok(kml) + } + + fn text_cb(&mut self, event: &BytesText, reader: &quick_xml::Reader) { + if 0 != self.tag & (0x4 | 0x10) { + let val = event.unescape_and_decode(reader).unwrap_or_default(); + + let val = val + .replace("\n", "") + .replace("\r", "") + .replace("\t", "") + .replace(" ", ""); + + if 0 != self.tag & 0x4 && val.len() >= 19 { + // YYYY-MM-DDTHH:MM:SSZ + // 0 4 7 10 13 16 19 + match chrono::NaiveDateTime::parse_from_str(&val, "%Y-%m-%dT%H:%M:%SZ") { + Ok(res) => { + self.curr.timestamp = res.timestamp(); + if self.curr.timestamp > time() { + self.curr.timestamp = time(); + } + } + Err(_err) => { + self.curr.timestamp = time(); + } + } + } else if 0 != self.tag & 0x10 { + let parts = val.splitn(2, ',').collect::>(); + if parts.len() == 2 { + self.curr.longitude = parts[0].parse().unwrap_or_default(); + self.curr.latitude = parts[1].parse().unwrap_or_default(); + } + } + } + } + + fn endtag_cb(&mut self, event: &BytesEnd) { + let tag = String::from_utf8_lossy(event.name()).trim().to_lowercase(); + + if tag == "placemark" { + if 0 != self.tag & 0x1 + && 0 != self.curr.timestamp + && 0. != self.curr.latitude + && 0. != self.curr.longitude + { + if let Some(ref mut locations) = self.locations { + locations.push(std::mem::replace(&mut self.curr, Location::new())); + } + } + self.tag = 0 + }; + } + + fn starttag_cb( + &mut self, + event: &BytesStart, + reader: &quick_xml::Reader, + ) { + let tag = String::from_utf8_lossy(event.name()).trim().to_lowercase(); + if tag == "document" { + if let Some(addr) = event.attributes().find(|attr| { + attr.as_ref() + .map(|a| String::from_utf8_lossy(a.key).trim().to_lowercase() == "addr") + .unwrap_or_default() + }) { + self.addr = addr.unwrap().unescape_and_decode_value(reader).ok(); + } + } else if tag == "placemark" { + self.tag = 0x1; + self.curr.timestamp = 0; + self.curr.latitude = 0.0; + self.curr.longitude = 0.0; + self.curr.accuracy = 0.0 + } else if tag == "timestamp" && 0 != self.tag & 0x1 { + self.tag = 0x1 | 0x2 + } else if tag == "when" && 0 != self.tag & 0x2 { + self.tag = 0x1 | 0x2 | 0x4 + } else if tag == "point" && 0 != self.tag & 0x1 { + self.tag = 0x1 | 0x8 + } else if tag == "coordinates" && 0 != self.tag & 0x8 { + self.tag = 0x1 | 0x8 | 0x10; + if let Some(acc) = event.attributes().find(|attr| { + attr.as_ref() + .map(|a| String::from_utf8_lossy(a.key).trim().to_lowercase() == "accuracy") + .unwrap_or_default() + }) { + let v = acc + .unwrap() + .unescape_and_decode_value(reader) + .unwrap_or_default(); + + self.curr.accuracy = v.trim().parse().unwrap_or_default(); + } + } + } } // location streaming -pub fn dc_send_locations_to_chat(context: &Context, chat_id: u32, seconds: i64) { +pub fn send_locations_to_chat(context: &Context, chat_id: u32, seconds: i64) { let now = time(); let mut msg: Message; let is_sending_locations_before: bool; if !(seconds < 0 || chat_id <= 9i32 as libc::c_uint) { - is_sending_locations_before = dc_is_sending_locations_to_chat(context, chat_id); + is_sending_locations_before = is_sending_locations_to_chat(context, chat_id); if sql::execute( context, &context.sql, @@ -101,9 +233,6 @@ pub fn dc_send_locations_to_chat(context: &Context, chat_id: u32, seconds: i64) } } -/******************************************************************************* - * job to send locations out to all chats that want them - ******************************************************************************/ #[allow(non_snake_case)] fn schedule_MAYBE_SEND_LOCATIONS(context: &Context, flags: i32) { if 0 != flags & 0x1 || !job_action_exists(context, Action::MaybeSendLocations) { @@ -111,7 +240,7 @@ fn schedule_MAYBE_SEND_LOCATIONS(context: &Context, flags: i32) { }; } -pub fn dc_is_sending_locations_to_chat(context: &Context, chat_id: u32) -> bool { +pub fn is_sending_locations_to_chat(context: &Context, chat_id: u32) -> bool { context .sql .exists( @@ -121,12 +250,7 @@ pub fn dc_is_sending_locations_to_chat(context: &Context, chat_id: u32) -> bool .unwrap_or_default() } -pub fn dc_set_location( - context: &Context, - latitude: f64, - longitude: f64, - accuracy: f64, -) -> libc::c_int { +pub fn set(context: &Context, latitude: f64, longitude: f64, accuracy: f64) -> libc::c_int { if latitude == 0.0 && longitude == 0.0 { return 1; } @@ -162,7 +286,7 @@ pub fn dc_set_location( ).unwrap_or_default() } -pub fn dc_get_locations( +pub fn get_range( context: &Context, chat_id: u32, contact_id: u32, @@ -229,15 +353,13 @@ fn is_marker(txt: &str) -> bool { txt.len() == 1 && txt.chars().next().unwrap() != ' ' } -pub fn dc_delete_all_locations(context: &Context) -> bool { - if sql::execute(context, &context.sql, "DELETE FROM locations;", params![]).is_err() { - return false; - } +pub fn delete_all(context: &Context) -> Result<(), Error> { + sql::execute(context, &context.sql, "DELETE FROM locations;", params![])?; context.call_cb(Event::LOCATION_CHANGED, 0, 0); - true + Ok(()) } -pub fn dc_get_location_kml(context: &Context, chat_id: u32) -> Result<(String, u32), Error> { +pub fn get_kml(context: &Context, chat_id: u32) -> Result<(String, u32), Error> { let now = time(); let mut location_count = 0; let mut ret = String::new(); @@ -313,7 +435,7 @@ fn get_kml_timestamp(utc: i64) -> String { .to_string() } -pub fn dc_get_message_kml(timestamp: i64, latitude: f64, longitude: f64) -> String { +pub fn get_message_kml(timestamp: i64, latitude: f64, longitude: f64) -> String { format!( "\n\ \n\ @@ -330,222 +452,84 @@ pub fn dc_get_message_kml(timestamp: i64, latitude: f64, longitude: f64) -> Stri ) } -pub fn dc_set_kml_sent_timestamp(context: &Context, chat_id: u32, timestamp: i64) -> bool { +pub fn set_kml_sent_timestamp( + context: &Context, + chat_id: u32, + timestamp: i64, +) -> Result<(), Error> { sql::execute( context, &context.sql, "UPDATE chats SET locations_last_sent=? WHERE id=?;", params![timestamp, chat_id as i32], - ) - .is_ok() + )?; + + Ok(()) } -pub fn dc_set_msg_location_id(context: &Context, msg_id: u32, location_id: u32) -> bool { +pub fn set_msg_location_id(context: &Context, msg_id: u32, location_id: u32) -> Result<(), Error> { sql::execute( context, &context.sql, "UPDATE msgs SET location_id=? WHERE id=?;", params![location_id, msg_id as i32], - ) - .is_ok() + )?; + + Ok(()) } -pub fn dc_save_locations( +pub fn save( context: &Context, chat_id: u32, contact_id: u32, - locations_opt: &Option>, + locations: &[Location], independent: i32, -) -> u32 { - if chat_id <= 9 || locations_opt.is_none() { - return 0; - } +) -> Result { + ensure!(chat_id > 9, "Invalid chat id"); + context.sql.prepare2( + "SELECT id FROM locations WHERE timestamp=? AND from_id=?", + "INSERT INTO locations\ + (timestamp, from_id, chat_id, latitude, longitude, accuracy, independent) \ + VALUES (?,?,?,?,?,?,?);", + |mut stmt_test, mut stmt_insert, conn| { + let mut newest_timestamp = 0; + let mut newest_location_id = 0; - let locations = locations_opt.as_ref().unwrap(); - context - .sql - .prepare2( - "SELECT id FROM locations WHERE timestamp=? AND from_id=?", - "INSERT INTO locations\ - (timestamp, from_id, chat_id, latitude, longitude, accuracy, independent) \ - VALUES (?,?,?,?,?,?,?);", - |mut stmt_test, mut stmt_insert, conn| { - let mut newest_timestamp = 0; - let mut newest_location_id = 0; + for location in locations { + let exists = stmt_test.exists(params![location.timestamp, contact_id as i32])?; - for location in locations { - let exists = - stmt_test.exists(params![location.timestamp, contact_id as i32])?; + if 0 != independent || !exists { + stmt_insert.execute(params![ + location.timestamp, + contact_id as i32, + chat_id as i32, + location.latitude, + location.longitude, + location.accuracy, + independent, + ])?; - if 0 != independent || !exists { - stmt_insert.execute(params![ + if location.timestamp > newest_timestamp { + newest_timestamp = location.timestamp; + newest_location_id = sql::get_rowid2_with_conn( + context, + conn, + "locations", + "timestamp", location.timestamp, + "from_id", contact_id as i32, - chat_id as i32, - location.latitude, - location.longitude, - location.accuracy, - independent, - ])?; - - if location.timestamp > newest_timestamp { - newest_timestamp = location.timestamp; - newest_location_id = sql::get_rowid2_with_conn( - context, - conn, - "locations", - "timestamp", - location.timestamp, - "from_id", - contact_id as i32, - ); - } + ); } } - Ok(newest_location_id) - }, - ) - .unwrap_or_default() -} - -pub fn dc_kml_parse(context: &Context, content: impl AsRef) -> Result { - ensure!( - content.as_ref().len() <= (1 * 1024 * 1024), - "A kml-files with {} bytes is larger than reasonably expected.", - content.as_ref().len() - ); - - let mut reader = quick_xml::Reader::from_str(content.as_ref()); - reader.trim_text(true); - - let mut kml = Kml::new(); - kml.locations = Some(Vec::with_capacity(100)); - - let mut buf = Vec::new(); - - loop { - match reader.read_event(&mut buf) { - Ok(quick_xml::events::Event::Start(ref e)) => kml_starttag_cb(e, &mut kml, &reader), - Ok(quick_xml::events::Event::End(ref e)) => kml_endtag_cb(e, &mut kml), - Ok(quick_xml::events::Event::Text(ref e)) => kml_text_cb(e, &mut kml, &reader), - Err(e) => { - error!( - context, - 0, - "Location parsing: Error at position {}: {:?}", - reader.buffer_position(), - e - ); } - Ok(quick_xml::events::Event::Eof) => break, - _ => (), - } - buf.clear(); - } - - Ok(kml) -} - -fn kml_text_cb( - event: &BytesText, - kml: &mut Kml, - reader: &quick_xml::Reader, -) { - if 0 != kml.tag & (0x4 | 0x10) { - let val = event.unescape_and_decode(reader).unwrap_or_default(); - - let val = val - .replace("\n", "") - .replace("\r", "") - .replace("\t", "") - .replace(" ", ""); - - if 0 != kml.tag & 0x4 && val.len() >= 19 { - // YYYY-MM-DDTHH:MM:SSZ - // 0 4 7 10 13 16 19 - match chrono::NaiveDateTime::parse_from_str(&val, "%Y-%m-%dT%H:%M:%SZ") { - Ok(res) => { - kml.curr.timestamp = res.timestamp(); - if kml.curr.timestamp > time() { - kml.curr.timestamp = time(); - } - } - Err(_err) => { - kml.curr.timestamp = time(); - } - } - } else if 0 != kml.tag & 0x10 { - let parts = val.splitn(2, ',').collect::>(); - if parts.len() == 2 { - kml.curr.longitude = parts[0].parse().unwrap_or_default(); - kml.curr.latitude = parts[1].parse().unwrap_or_default(); - } - } - } -} - -fn kml_endtag_cb(event: &BytesEnd, kml: &mut Kml) { - let tag = String::from_utf8_lossy(event.name()).trim().to_lowercase(); - - if tag == "placemark" { - if 0 != kml.tag & 0x1 - && 0 != kml.curr.timestamp - && 0. != kml.curr.latitude - && 0. != kml.curr.longitude - { - if let Some(ref mut locations) = kml.locations { - locations.push(std::mem::replace(&mut kml.curr, Location::new())); - } - } - kml.tag = 0 - }; -} - -fn kml_starttag_cb( - event: &BytesStart, - kml: &mut Kml, - reader: &quick_xml::Reader, -) { - let tag = String::from_utf8_lossy(event.name()).trim().to_lowercase(); - if tag == "document" { - if let Some(addr) = event.attributes().find(|attr| { - attr.as_ref() - .map(|a| String::from_utf8_lossy(a.key).trim().to_lowercase() == "addr") - .unwrap_or_default() - }) { - kml.addr = addr.unwrap().unescape_and_decode_value(reader).ok(); - } - } else if tag == "placemark" { - kml.tag = 0x1; - kml.curr.timestamp = 0; - kml.curr.latitude = 0.0; - kml.curr.longitude = 0.0; - kml.curr.accuracy = 0.0 - } else if tag == "timestamp" && 0 != kml.tag & 0x1 { - kml.tag = 0x1 | 0x2 - } else if tag == "when" && 0 != kml.tag & 0x2 { - kml.tag = 0x1 | 0x2 | 0x4 - } else if tag == "point" && 0 != kml.tag & 0x1 { - kml.tag = 0x1 | 0x8 - } else if tag == "coordinates" && 0 != kml.tag & 0x8 { - kml.tag = 0x1 | 0x8 | 0x10; - if let Some(acc) = event.attributes().find(|attr| { - attr.as_ref() - .map(|a| String::from_utf8_lossy(a.key).trim().to_lowercase() == "accuracy") - .unwrap_or_default() - }) { - let v = acc - .unwrap() - .unescape_and_decode_value(reader) - .unwrap_or_default(); - - kml.curr.accuracy = v.trim().parse().unwrap_or_default(); - } - } + Ok(newest_location_id) + }, + ) } #[allow(non_snake_case)] -pub fn dc_job_do_DC_JOB_MAYBE_SEND_LOCATIONS(context: &Context, _job: &Job) { +pub fn job_do_DC_JOB_MAYBE_SEND_LOCATIONS(context: &Context, _job: &Job) { let now = time(); let mut continue_streaming: libc::c_int = 1; info!( @@ -626,7 +610,7 @@ pub fn dc_job_do_DC_JOB_MAYBE_SEND_LOCATIONS(context: &Context, _job: &Job) { } #[allow(non_snake_case)] -pub fn dc_job_do_DC_JOB_MAYBE_SEND_LOC_ENDED(context: &Context, job: &mut Job) { +pub fn job_do_DC_JOB_MAYBE_SEND_LOC_ENDED(context: &Context, job: &mut Job) { // this function is called when location-streaming _might_ have ended for a chat. // the function checks, if location-streaming is really ended; // if so, a device-message is added if not yet done. @@ -667,13 +651,13 @@ mod tests { use crate::test_utils::dummy_context; #[test] - fn test_dc_kml_parse() { + fn test_kml_parse() { let context = dummy_context(); let xml = "\n\n\n2019-03-06T21:09:57Z9.423110,53.790302\n\n \n\t2018-12-13T22:11:12Z\t 19.423110 \t , \n 63.790302\n \n\n"; - let kml = dc_kml_parse(&context.ctx, &xml).expect("parsing failed"); + let kml = Kml::parse(&context.ctx, &xml).expect("parsing failed"); assert!(kml.addr.is_some()); assert_eq!(kml.addr.as_ref().unwrap(), "user@example.org",); From 4ec214b3fc915a894286bdc50b9bae3db7c8a6d9 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 26 Aug 2019 21:46:57 +0200 Subject: [PATCH 4/6] refactor(location): use bitflags for tags --- Cargo.lock | 1 + Cargo.toml | 1 + src/location.rs | 43 ++++++++++++++++++++++++++++--------------- 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 189c64ea4..10a2d8490 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -463,6 +463,7 @@ version = "1.0.0-alpha.4" dependencies = [ "backtrace 0.3.34 (registry+https://github.com/rust-lang/crates.io-index)", "base64 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "cc 1.0.40 (registry+https://github.com/rust-lang/crates.io-index)", "charset 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index d5204145c..825081cd0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,6 +49,7 @@ itertools = "0.8.0" image-meta = "0.1.0" quick-xml = "0.15.0" escaper = "0.1.0" +bitflags = "1.1.0" [dev-dependencies] tempfile = "3.0" diff --git a/src/location.rs b/src/location.rs index abfd4101f..08e641921 100644 --- a/src/location.rs +++ b/src/location.rs @@ -1,3 +1,4 @@ +use bitflags::bitflags; use quick_xml; use quick_xml::events::{BytesEnd, BytesStart, BytesText}; @@ -39,10 +40,22 @@ impl Location { pub struct Kml { pub addr: Option, pub locations: Option>, - pub tag: i32, + tag: KmlTag, pub curr: Location, } +bitflags! { + #[derive(Default)] + struct KmlTag: i32 { + const UNDEFINED = 0x00; + const PLACEMARK = 0x01; + const TIMESTAMP = 0x02; + const WHEN = 0x04; + const POINT = 0x08; + const COORDINATES = 0x10; + } +} + impl Kml { pub fn new() -> Self { Default::default() @@ -87,7 +100,7 @@ impl Kml { } fn text_cb(&mut self, event: &BytesText, reader: &quick_xml::Reader) { - if 0 != self.tag & (0x4 | 0x10) { + if self.tag.contains(KmlTag::WHEN) || self.tag.contains(KmlTag::COORDINATES) { let val = event.unescape_and_decode(reader).unwrap_or_default(); let val = val @@ -96,7 +109,7 @@ impl Kml { .replace("\t", "") .replace(" ", ""); - if 0 != self.tag & 0x4 && val.len() >= 19 { + if self.tag.contains(KmlTag::WHEN) && val.len() >= 19 { // YYYY-MM-DDTHH:MM:SSZ // 0 4 7 10 13 16 19 match chrono::NaiveDateTime::parse_from_str(&val, "%Y-%m-%dT%H:%M:%SZ") { @@ -110,7 +123,7 @@ impl Kml { self.curr.timestamp = time(); } } - } else if 0 != self.tag & 0x10 { + } else if self.tag.contains(KmlTag::COORDINATES) { let parts = val.splitn(2, ',').collect::>(); if parts.len() == 2 { self.curr.longitude = parts[0].parse().unwrap_or_default(); @@ -124,7 +137,7 @@ impl Kml { let tag = String::from_utf8_lossy(event.name()).trim().to_lowercase(); if tag == "placemark" { - if 0 != self.tag & 0x1 + if self.tag.contains(KmlTag::PLACEMARK) && 0 != self.curr.timestamp && 0. != self.curr.latitude && 0. != self.curr.longitude @@ -133,7 +146,7 @@ impl Kml { locations.push(std::mem::replace(&mut self.curr, Location::new())); } } - self.tag = 0 + self.tag = KmlTag::UNDEFINED; }; } @@ -152,19 +165,19 @@ impl Kml { self.addr = addr.unwrap().unescape_and_decode_value(reader).ok(); } } else if tag == "placemark" { - self.tag = 0x1; + self.tag = KmlTag::PLACEMARK; self.curr.timestamp = 0; self.curr.latitude = 0.0; self.curr.longitude = 0.0; self.curr.accuracy = 0.0 - } else if tag == "timestamp" && 0 != self.tag & 0x1 { - self.tag = 0x1 | 0x2 - } else if tag == "when" && 0 != self.tag & 0x2 { - self.tag = 0x1 | 0x2 | 0x4 - } else if tag == "point" && 0 != self.tag & 0x1 { - self.tag = 0x1 | 0x8 - } else if tag == "coordinates" && 0 != self.tag & 0x8 { - self.tag = 0x1 | 0x8 | 0x10; + } else if tag == "timestamp" && self.tag.contains(KmlTag::PLACEMARK) { + self.tag = KmlTag::PLACEMARK | KmlTag::TIMESTAMP + } else if tag == "when" && self.tag.contains(KmlTag::TIMESTAMP) { + self.tag = KmlTag::PLACEMARK | KmlTag::TIMESTAMP | KmlTag::WHEN + } else if tag == "point" && self.tag.contains(KmlTag::PLACEMARK) { + self.tag = KmlTag::PLACEMARK | KmlTag::POINT + } else if tag == "coordinates" && self.tag.contains(KmlTag::POINT) { + self.tag = KmlTag::PLACEMARK | KmlTag::POINT | KmlTag::COORDINATES; if let Some(acc) = event.attributes().find(|attr| { attr.as_ref() .map(|a| String::from_utf8_lossy(a.key).trim().to_lowercase() == "accuracy") From 3f8abd22185843b8c229c69a5e7afcccc3ad014d Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 26 Aug 2019 21:49:47 +0200 Subject: [PATCH 5/6] refactor(location): make location on the kml object not optional --- src/dc_receive_imf.rs | 42 ++++++++++++++++++------------------------ src/location.rs | 11 +++++------ 2 files changed, 23 insertions(+), 30 deletions(-) diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index f3764d45b..d3f5cc7cd 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -938,15 +938,13 @@ unsafe fn save_locations( let mut send_event = false; if !mime_parser.message_kml.is_none() && chat_id > DC_CHAT_ID_LAST_SPECIAL as libc::c_uint { - if let Some(ref locations) = mime_parser.message_kml.as_ref().unwrap().locations { - let newest_location_id = - location::save(context, chat_id, from_id, locations, 1).unwrap_or_default(); - if 0 != newest_location_id && 0 == hidden { - if location::set_msg_location_id(context, insert_msg_id, newest_location_id).is_ok() - { - location_id_written = true; - send_event = true; - } + let locations = &mime_parser.message_kml.as_ref().unwrap().locations; + let newest_location_id = + location::save(context, chat_id, from_id, locations, 1).unwrap_or_default(); + if 0 != newest_location_id && 0 == hidden { + if location::set_msg_location_id(context, insert_msg_id, newest_location_id).is_ok() { + location_id_written = true; + send_event = true; } } } @@ -957,23 +955,19 @@ unsafe fn save_locations( if !contact.get_addr().is_empty() && contact.get_addr().to_lowercase() == addr.to_lowercase() { - if let Some(ref locations) = - mime_parser.location_kml.as_ref().unwrap().locations - { - let newest_location_id = - location::save(context, chat_id, from_id, locations, 0) - .unwrap_or_default(); - if newest_location_id != 0 && hidden == 0 && !location_id_written { - if let Err(err) = location::set_msg_location_id( - context, - insert_msg_id, - newest_location_id, - ) { - error!(context, 0, "Failed to set msg_location_id: {:?}", err); - } + let locations = &mime_parser.location_kml.as_ref().unwrap().locations; + let newest_location_id = + location::save(context, chat_id, from_id, locations, 0).unwrap_or_default(); + if newest_location_id != 0 && hidden == 0 && !location_id_written { + if let Err(err) = location::set_msg_location_id( + context, + insert_msg_id, + newest_location_id, + ) { + error!(context, 0, "Failed to set msg_location_id: {:?}", err); } - send_event = true; } + send_event = true; } } } diff --git a/src/location.rs b/src/location.rs index 08e641921..66444d948 100644 --- a/src/location.rs +++ b/src/location.rs @@ -39,7 +39,7 @@ impl Location { #[derive(Debug, Clone, Default)] pub struct Kml { pub addr: Option, - pub locations: Option>, + pub locations: Vec, tag: KmlTag, pub curr: Location, } @@ -72,7 +72,7 @@ impl Kml { reader.trim_text(true); let mut kml = Kml::new(); - kml.locations = Some(Vec::with_capacity(100)); + kml.locations = Vec::with_capacity(100); let mut buf = Vec::new(); @@ -142,9 +142,8 @@ impl Kml { && 0. != self.curr.latitude && 0. != self.curr.longitude { - if let Some(ref mut locations) = self.locations { - locations.push(std::mem::replace(&mut self.curr, Location::new())); - } + self.locations + .push(std::mem::replace(&mut self.curr, Location::new())); } self.tag = KmlTag::UNDEFINED; }; @@ -675,7 +674,7 @@ mod tests { assert!(kml.addr.is_some()); assert_eq!(kml.addr.as_ref().unwrap(), "user@example.org",); - let locations_ref = &kml.locations.as_ref().unwrap(); + let locations_ref = &kml.locations; assert_eq!(locations_ref.len(), 2); assert!(locations_ref[0].latitude > 53.6f64); From 25b0a26ff9fcfe3e2e33463821449ed63a0b0031 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 26 Aug 2019 21:50:33 +0200 Subject: [PATCH 6/6] fix(ffi): handle result from location deletion --- deltachat-ffi/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index ca255ae2b..b7b1621d4 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -1096,7 +1096,7 @@ pub unsafe extern "C" fn dc_delete_all_locations(context: *mut dc_context_t) { assert!(!context.is_null()); let context = &*context; - location::delete_all(context); + location::delete_all(context).log_err(context, "Failed to delete locations"); } // dc_array_t