Bug 1433529: Fixed TODOs in parse_sdp, r=dminor
Changed parse_sdp to use StringView instad of raw pointers
Fixed all TODOs by using the existing StringView::into trait instead of doing a manual string conversion.
MozReview-Commit-ID: 5m7rLAu8vwS
--- a/media/webrtc/signaling/src/sdp/RsdparsaSdpInc.h
+++ b/media/webrtc/signaling/src/sdp/RsdparsaSdpInc.h
@@ -251,17 +251,17 @@ nsresult u32_vec_get(const U32Vec* vec,
size_t u16_vec_len(const U16Vec* vec);
nsresult u16_vec_get(const U16Vec* vec, size_t index, uint16_t* ret);
size_t u8_vec_len(const U8Vec* vec);
nsresult u8_vec_get(const U8Vec* vec, size_t index, uint8_t* ret);
void sdp_free_string(char* string);
-nsresult parse_sdp(const char* sdp, uint32_t length, bool fail_on_warning,
+nsresult parse_sdp(StringView sdp, bool fail_on_warning,
RustSdpSession** ret, RustSdpError** err);
RustSdpSession* sdp_new_reference(RustSdpSession* aSess);
void sdp_free_session(RustSdpSession* ret);
size_t sdp_get_error_line_num(const RustSdpError* err);
StringView sdp_get_error_message(const RustSdpError* err);
void sdp_free_error(RustSdpError* err);
RustSdpOrigin sdp_get_origin(const RustSdpSession* aSess);
--- a/media/webrtc/signaling/src/sdp/RsdparsaSdpParser.cpp
+++ b/media/webrtc/signaling/src/sdp/RsdparsaSdpParser.cpp
@@ -17,32 +17,25 @@
namespace mozilla
{
UniquePtr<Sdp>
RsdparsaSdpParser::Parse(const std::string &sdpText)
{
ClearParseErrors();
- const char* rawString = sdpText.c_str();
RustSdpSession* result;
RustSdpError* err;
- nsresult rv = parse_sdp(rawString, sdpText.length() + 1, false,
- &result, &err);
+ StringView sdpTextView{sdpText.c_str(), sdpText.length()};
+ nsresult rv = parse_sdp(sdpTextView, false, &result, &err);
if (rv != NS_OK) {
- // TODO: err should eventually never be null if rv != NS_OK
- // see Bug 1433529
- if (err != nullptr) {
- size_t line = sdp_get_error_line_num(err);
- std::string errMsg = convertStringView(sdp_get_error_message(err));
- sdp_free_error(err);
- AddParseError(line, errMsg);
- } else {
- AddParseError(0, "Unhandled Rsdparsa parsing error");
- }
+ size_t line = sdp_get_error_line_num(err);
+ std::string errMsg = convertStringView(sdp_get_error_message(err));
+ sdp_free_error(err);
+ AddParseError(line, errMsg);
return nullptr;
}
if(err) {
size_t line = sdp_get_error_line_num(err);
std::string warningMsg = convertStringView(sdp_get_error_message(err));
sdp_free_error(err);
AddParseWarnings(line, warningMsg);
--- a/media/webrtc/signaling/src/sdp/rsdparsa_capi/src/lib.rs
+++ b/media/webrtc/signaling/src/sdp/rsdparsa_capi/src/lib.rs
@@ -1,15 +1,14 @@
extern crate rsdparsa;
extern crate libc;
#[macro_use] extern crate log;
extern crate nserror;
-use std::ffi::CStr;
-use std::{str, slice, ptr};
+use std::ptr;
use std::os::raw::c_char;
use std::error::Error;
use libc::size_t;
use std::rc::Rc;
use nserror::{nsresult, NS_OK, NS_ERROR_INVALID_ARG};
@@ -23,44 +22,33 @@ pub mod network;
pub mod attribute;
pub mod media_section;
pub use types::{StringView, NULL_STRING};
use network::{RustSdpOrigin, origin_view_helper, RustSdpConnection,
get_bandwidth};
#[no_mangle]
-pub unsafe extern "C" fn parse_sdp(sdp: *const u8, length: u32,
+pub unsafe extern "C" fn parse_sdp(sdp: StringView,
fail_on_warning: bool,
session: *mut *const SdpSession,
error: *mut *const SdpParserError) -> nsresult {
- // Bug 1433529 tracks fixing the TODOs in this function.
- // TODO: Do I need to add explicit lifetime here?
- // https://gankro.github.io/blah/only-in-rust/#honorable-mention-variance
- let sdp_slice: &[u8] = slice::from_raw_parts(sdp, length as usize);
- let sdp_c_str = match CStr::from_bytes_with_nul(sdp_slice) {
+ let sdp_str = match sdp.into() {
Ok(string) => string,
- Err(_) => {
+ Err(boxed_error) => {
*session = ptr::null();
- *error = ptr::null(); // TODO: Give more useful return value here
- debug!("Error converting string");
+ *error = Box::into_raw(Box::new(SdpParserError::Sequence {
+ message: (*boxed_error).description().to_string(),
+ line_number: 0,
+ }));
return NS_ERROR_INVALID_ARG;
}
};
- let sdp_buf: &[u8] = sdp_c_str.to_bytes();
- let sdp_str_slice: &str = match str::from_utf8(sdp_buf) {
- Ok(string) => string,
- Err(_) => {
- *session = ptr::null();
- *error = ptr::null(); // TODO: Give more useful return value here
- debug!("Error converting string to utf8");
- return NS_ERROR_INVALID_ARG;
- }
- };
- let parser_result = rsdparsa::parse_sdp(sdp_str_slice, fail_on_warning);
+
+ let parser_result = rsdparsa::parse_sdp(&sdp_str, fail_on_warning);
match parser_result {
Ok(parsed) => {
*error = match parsed.warnings.len(){
0 => ptr::null(),
_ => Box::into_raw(Box::new(parsed.warnings[0].clone())),
};
*session = Rc::into_raw(Rc::new(parsed));
NS_OK