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

--HG--
extra : rebase_source : f34cf389829772b4ca84d7ba895a8b71bd620f64
This commit is contained in:
Johannes Willbold 2018-06-29 15:03:58 -07:00
parent 5bdc727943
commit 0e35e80e48
3 changed files with 17 additions and 36 deletions

View File

@ -256,7 +256,7 @@ 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);

View File

@ -22,22 +22,15 @@ 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;
}

View File

@ -3,8 +3,7 @@ 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;
@ -28,34 +27,23 @@ 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(){