From 0808e1b8dc84c4ad3005234f7cf1d82edbe8a1eb Mon Sep 17 00:00:00 2001 From: Stephen Crane Date: Fri, 7 Feb 2020 12:07:59 -0800 Subject: [PATCH 01/14] Convert parser m_buffer field to a Rust managed Vec Along with converting the backing storage, this commit also changes some pointers into the buffer into indices: m_bufferPtr: *const c_char -> m_bufferStart: usize m_bufferEnd: *mut c_char -> m_bufferEnd: usize m_positionPtr: *const c_char -> m_positionIdx: usize There are still more pointers that need to be converted as part of this change, but this is a minimal working set of changes. --- src/lib/xmlparse.rs | 238 ++++++++++++++++++------------------------ src/tests/runtests.rs | 10 +- 2 files changed, 110 insertions(+), 138 deletions(-) diff --git a/src/lib/xmlparse.rs b/src/lib/xmlparse.rs index 05a84c1d..7387f5ee 100644 --- a/src/lib/xmlparse.rs +++ b/src/lib/xmlparse.rs @@ -770,13 +770,11 @@ pub struct XML_ParserStruct { /* The first member must be m_userData so that the XML_GetUserData macro works. */ pub m_userData: *mut c_void, - pub m_buffer: *mut c_char, - /* first character to be parsed */ - pub m_bufferPtr: *const c_char, - /* past last character to be parsed */ - pub m_bufferEnd: *mut c_char, - /* allocated end of m_buffer */ - pub m_bufferLim: *const c_char, + m_buffer: Vec, + // index of first character to be parsed + m_bufferStart: usize, + // index after last character to be parsed + m_bufferEnd: usize, pub m_parseEndByteIndex: XML_Index, pub m_parseEndPtr: *const c_char, pub m_dataBuf: *mut XML_Char, // Box<[XML_Char; INIT_DATA_BUF_SIZE]> @@ -795,7 +793,7 @@ pub struct XML_ParserStruct { pub m_errorCode: XML_Error, pub m_eventPtr: *const c_char, pub m_eventEndPtr: *const c_char, - pub m_positionPtr: *const c_char, + pub m_positionIdx: usize, pub m_openInternalEntities: *mut OpenInternalEntity, pub m_freeInternalEntities: *mut OpenInternalEntity, pub m_defaultExpandInternalEntities: XML_Bool, @@ -1625,13 +1623,11 @@ impl XML_ParserStruct { fn try_new(use_namespaces: bool, dtd: Rc) -> Result { Ok(Self { m_userData: ptr::null_mut(), - m_buffer: ptr::null_mut(), - /* first character to be parsed */ - m_bufferPtr: ptr::null(), - /* past last character to be parsed */ - m_bufferEnd: ptr::null_mut(), - /* allocated end of m_buffer */ - m_bufferLim: ptr::null(), + m_buffer: Vec::new(), + // index of first character to be parsed + m_bufferStart: 0, + // index after last character to be parsed + m_bufferEnd: 0, m_parseEndByteIndex: 0, m_parseEndPtr: ptr::null(), m_dataBuf: ptr::null_mut(), // Box<[XML_Char; INIT_DATA_BUF_SIZE]> @@ -1664,7 +1660,7 @@ impl XML_ParserStruct { m_errorCode: XML_Error::NONE, m_eventPtr: ptr::null(), m_eventEndPtr: ptr::null(), - m_positionPtr: ptr::null(), + m_positionIdx: 0, m_openInternalEntities: ptr::null_mut(), m_freeInternalEntities: ptr::null_mut(), m_defaultExpandInternalEntities: false, @@ -1714,19 +1710,12 @@ impl XML_ParserStruct { mut dtd: Rc, ) -> XML_Parser { let use_namespaces = !nameSep.is_null(); - let mut parser = match XML_ParserStruct::try_new(use_namespaces, dtd) { - Ok(parser) => parser, - Err(()) => return ptr::null_mut(), - }; - let mut parser = match Box::try_new(parser) { Ok(p) => p, Err(_) => return ptr::null_mut(), }; - // TODO: Move initialization into XML_ParserStruct::try_new - parser.m_buffer = ptr::null_mut(); - parser.m_bufferLim = ptr::null(); + // TODO: Move initialization into XML_ParserStruct::new if parser.m_atts.try_reserve(INIT_ATTS_SIZE as usize).is_err() { return ptr::null_mut(); } @@ -1777,9 +1766,9 @@ impl XML_ParserStruct { self.m_userData = ptr::null_mut(); self.m_handlers = Default::default(); self.m_handlers.m_externalEntityRefHandlerArg = self as XML_Parser; - self.m_bufferPtr = self.m_buffer; - self.m_bufferEnd = self.m_buffer; - self.m_parseEndByteIndex = 0; + self.m_bufferStart = 0; + self.m_bufferEnd = 0; + self.m_parseEndByteIndex = 0i64; self.m_parseEndPtr = ptr::null(); self.m_declElementType = ptr::null_mut(); self.m_declAttributeId = ptr::null_mut(); @@ -1800,7 +1789,7 @@ impl XML_ParserStruct { self.m_errorCode = XML_Error::NONE; self.m_eventPtr = ptr::null(); self.m_eventEndPtr = ptr::null(); - self.m_positionPtr = ptr::null(); + self.m_positionIdx = 0; self.m_openInternalEntities = ptr::null_mut(); self.m_defaultExpandInternalEntities = true; self.m_tagLevel = 0; @@ -2083,7 +2072,6 @@ impl Drop for XML_ParserStruct { parser->m_dtd with the root parser, so we must not destroy it */ FREE!(self.m_groupConnector); - FREE!(self.m_buffer); FREE!(self.m_dataBuf); if self.m_handlers.m_unknownEncodingRelease.is_some() { self.m_handlers.m_unknownEncodingRelease @@ -2621,20 +2609,19 @@ impl XML_ParserStruct { if isFinal == 0 { return XML_Status::OK; } - self.m_positionPtr = self.m_bufferPtr; - self.m_parseEndPtr = self.m_bufferEnd; + self.m_positionIdx = self.m_bufferStart; + self.m_parseEndPtr = self.m_buffer.as_ptr().add(self.m_bufferEnd); /* If data are left over from last buffer, and we now know that these data are the final chunk of input, then we have to check them again to detect errors based on that fact. - */ + */ + let mut start_ptr = self.m_buffer.as_ptr().add(self.m_bufferStart); self.m_errorCode = self.m_processor.expect("non-null function pointer")( self, - ExpatBufRef::new( - self.m_bufferPtr, - self.m_parseEndPtr, - ), - &mut self.m_bufferPtr, + self.m_buffer[self.m_bufferStart..self.m_bufferEnd].into(), + &mut start_ptr, ); + self.m_bufferStart = start_ptr.wrapping_offset_from(self.m_buffer.as_ptr()) as usize; if self.m_errorCode == XML_Error::NONE { match self.m_parsingStatus.parsing { XML_Parsing::SUSPENDED => { @@ -2651,13 +2638,10 @@ impl XML_ParserStruct { * LCOV_EXCL_START */ (*self.m_encoding).updatePosition( - ExpatBufRef::new( - self.m_positionPtr, - self.m_bufferPtr, - ), + self.m_buffer[self.m_positionIdx..self.m_bufferStart].into(), &mut self.m_position, ); - self.m_positionPtr = self.m_bufferPtr; + self.m_positionIdx = self.m_bufferStart; return XML_Status::SUSPENDED; } XML_Parsing::INITIALIZED | XML_Parsing::PARSING => { @@ -2674,12 +2658,11 @@ impl XML_ParserStruct { XML_Status::ERROR } else { /* not defined XML_CONTEXT_BYTES */ - let mut buff: *mut c_void = self.getBuffer(len); - if buff.is_null() { - XML_Status::ERROR as XML_Status + if let Some(buff) = self.getBuffer(len) { + buff[..len as usize].copy_from_slice(std::slice::from_raw_parts(s, len as usize)); + XML_ParseBuffer(self, len, isFinal) } else { - memcpy(buff, s as *const c_void, len as usize); - self.parseBuffer(len, isFinal) + XML_Status::ERROR } } } @@ -2724,21 +2707,24 @@ impl XML_ParserStruct { } /* fall through */ self.m_parsingStatus.parsing = XML_Parsing::PARSING; - start = self.m_bufferPtr; - self.m_positionPtr = start; - self.m_bufferEnd = self.m_bufferEnd.offset(len as isize); - self.m_parseEndPtr = self.m_bufferEnd; + // TODO(SJC): is signed overflow an issue here? + start = self.m_buffer.as_ptr().add(self.m_bufferStart); + self.m_positionIdx = self.m_bufferStart; + self.m_bufferEnd += len as usize; + // TODO(SJC): is signed overflow an issue here? + self.m_parseEndPtr = self.m_buffer.as_ptr().add(self.m_bufferEnd); self.m_parseEndByteIndex += len as c_long; self.m_parsingStatus.finalBuffer = isFinal != 0; self.m_errorCode = self.m_processor.expect("non-null function pointer")( self, - ExpatBufRef::new( - start, - self.m_parseEndPtr, - ), - &mut self.m_bufferPtr, + self.m_buffer[self.m_bufferStart..self.m_bufferEnd].into(), + &mut start, ); +<<<<<<< HEAD +======= + self.m_bufferStart = start.wrapping_offset_from(self.m_buffer.as_ptr()) as usize; +>>>>>>> Convert parser m_buffer field to a Rust managed Vec if self.m_errorCode != XML_Error::NONE { self.m_eventEndPtr = self.m_eventPtr; self.m_processor = Some(errorProcessor as Processor); @@ -2759,14 +2745,10 @@ impl XML_ParserStruct { } } (*self.m_encoding).updatePosition( - ExpatBufRef::new( - self.m_positionPtr, - self.m_bufferPtr, - ), + self.m_buffer[self.m_positionIdx..self.m_bufferStart].into(), &mut self.m_position, ); - self.m_positionPtr = self.m_bufferPtr; - + self.m_positionIdx = self.m_bufferStart; result } } @@ -2784,102 +2766,85 @@ pub unsafe extern "C" fn XML_ParseBuffer( (*parser).parseBuffer(len, isFinal) } +<<<<<<< HEAD impl XML_ParserStruct { pub unsafe fn getBuffer(&mut self, len: c_int) -> *mut c_void { +======= +impl <'scf> XML_ParserStruct<'scf> { + pub unsafe fn getBuffer(&mut self, len: c_int) -> Option<&mut [c_char]> { +>>>>>>> Convert parser m_buffer field to a Rust managed Vec if len < 0 { self.m_errorCode = XML_Error::NO_MEMORY; - return ptr::null_mut(); + return None; } match self.m_parsingStatus.parsing { XML_Parsing::SUSPENDED => { self.m_errorCode = XML_Error::SUSPENDED; - return ptr::null_mut(); + return None; } XML_Parsing::FINISHED => { self.m_errorCode = XML_Error::FINISHED; - return ptr::null_mut(); + return None; } _ => {} } - if len as isize > safe_ptr_diff(self.m_bufferLim, self.m_bufferEnd) { - let maybe_needed_size = len.checked_add( - safe_ptr_diff(self.m_bufferEnd, self.m_bufferPtr) as c_int - ); + if len as usize > self.m_buffer.len() - self.m_bufferEnd { + let maybe_needed_size = len.checked_add((self.m_bufferEnd - self.m_bufferStart).try_into().unwrap()); let mut neededSize = match maybe_needed_size { None => { self.m_errorCode = XML_Error::NO_MEMORY; - return ptr::null_mut(); + return None; } - Some(s) => s, + Some(s) => s as usize, }; let keep = cmp::min( - XML_CONTEXT_BYTES, - safe_ptr_diff(self.m_bufferPtr, self.m_buffer) as c_int, + XML_CONTEXT_BYTES as usize, + self.m_bufferStart, ); neededSize += keep; - if (neededSize as isize) <= safe_ptr_diff(self.m_bufferLim, self.m_buffer) { - if (keep as isize) < safe_ptr_diff(self.m_bufferPtr, self.m_buffer) { - let offset = safe_ptr_diff(self.m_bufferPtr, self.m_buffer) - keep as isize; + if (neededSize as usize) <= self.m_buffer.len() { + if (keep as usize) < self.m_bufferStart { + let offset = self.m_bufferStart - keep as usize; /* The buffer pointers cannot be NULL here; we have at least some bytes * in the buffer */ - memmove( - self.m_buffer as *mut c_void, - &mut *self.m_buffer.offset(offset as isize) as *mut c_char - as *const c_void, - (self - .m_bufferEnd - .wrapping_offset_from(self.m_bufferPtr) - + keep as isize).try_into().unwrap(), - ); - self.m_bufferEnd = self.m_bufferEnd.offset(-offset); - self.m_bufferPtr = self.m_bufferPtr.offset(-offset); + self.m_buffer.copy_within(offset..self.m_bufferEnd, 0); + self.m_bufferEnd -= offset; + self.m_bufferStart -= offset; } } else { - let mut bufferSize: c_int = match safe_ptr_diff(self.m_bufferLim, self.m_bufferPtr) { + let mut bufferSize: c_int = match self.m_buffer.len() - self.m_bufferStart { 0 => INIT_BUFFER_SIZE, size => size.try_into().unwrap(), }; - while bufferSize < neededSize { + while (bufferSize as usize) < neededSize { bufferSize = match 2i32.checked_mul(bufferSize) { Some(s) => s, None => { self.m_errorCode = XML_Error::NO_MEMORY; - return ptr::null_mut(); + return None; } } } - let newBuf = MALLOC![c_char; bufferSize]; - if newBuf.is_null() { + if self.m_buffer.try_reserve(bufferSize as usize).is_err() { self.m_errorCode = XML_Error::NO_MEMORY; - return ptr::null_mut(); - } - self.m_bufferLim = newBuf.offset(bufferSize as isize); - if !self.m_bufferPtr.is_null() { - memcpy( - newBuf as *mut c_void, - &*self.m_bufferPtr.offset(-keep as isize) as *const c_char - as *const c_void, - safe_ptr_diff(self.m_bufferEnd, self.m_bufferPtr) as usize + keep as usize, - ); - FREE!(self.m_buffer); - self.m_buffer = newBuf; - self.m_bufferEnd = self - .m_buffer - .offset(safe_ptr_diff(self.m_bufferEnd, self.m_bufferPtr)) - .offset(keep as isize); - self.m_bufferPtr = self.m_buffer.offset(keep as isize) + return None; + } + self.m_buffer.resize(bufferSize as usize, 0); + if self.m_bufferStart < self.m_bufferEnd { + self.m_buffer.copy_within(self.m_bufferStart-keep..self.m_bufferEnd, 0); + self.m_bufferEnd = self.m_bufferEnd - self.m_bufferStart + keep; + self.m_bufferStart = keep; } else { /* This must be a brand new buffer with no data in it yet */ - self.m_bufferEnd = newBuf; - self.m_buffer = newBuf; - self.m_bufferPtr = self.m_buffer + self.m_bufferStart = 0; + self.m_bufferEnd = 0; } } self.m_eventEndPtr = ptr::null(); self.m_eventPtr = ptr::null(); - self.m_positionPtr = ptr::null(); + self.m_positionIdx = 0; } - self.m_bufferEnd as *mut c_void + Some(&mut self.m_buffer[self.m_bufferEnd..]) } } @@ -2889,7 +2854,11 @@ pub unsafe extern "C" fn XML_GetBuffer(mut parser: XML_Parser, mut len: c_int) - return ptr::null_mut(); } - (*parser).getBuffer(len) + if let Some(buf) = (*parser).getBuffer(len) { + buf.as_mut_ptr() as *mut c_void + } else { + ptr::null_mut() + } } /* Stops parsing, causing XML_Parse() or XML_ParseBuffer() to return. Must be called from within a call-back handler, except when aborting @@ -2983,10 +2952,10 @@ impl XML_ParserStruct { self.m_errorCode = self.m_processor.expect("non-null function pointer")( self, ExpatBufRef::new( - self.m_bufferPtr, + &self.m_buffer[self.m_bufferStart], self.m_parseEndPtr, ), - &mut self.m_bufferPtr, + &mut (&self.m_buffer[self.m_bufferStart] as *const _), ); if self.m_errorCode != XML_Error::NONE { self.m_eventEndPtr = self.m_eventPtr; @@ -3005,18 +2974,15 @@ impl XML_ParserStruct { } } (*self.m_encoding).updatePosition( - ExpatBufRef::new( - self.m_positionPtr, - self.m_bufferPtr, - ), + self.m_buffer[self.m_positionIdx..self.m_bufferStart].into(), &mut self.m_position, ); - self.m_positionPtr = self.m_bufferPtr; + self.m_positionIdx = self.m_bufferStart; #[cfg(feature = "mozilla")] { - self.m_eventPtr = self.m_bufferPtr; - self.m_eventEndPtr = self.m_bufferPtr; + self.m_eventPtr = &self.m_buffer[self.m_bufferStart]; + self.m_eventEndPtr = &self.m_buffer[self.m_bufferStart]; } result @@ -3109,18 +3075,16 @@ pub unsafe extern "C" fn XML_GetInputContext( if parser.is_null() { return ptr::null(); } - if !(*parser).m_eventPtr.is_null() && !(*parser).m_buffer.is_null() { + if !(*parser).m_eventPtr.is_null() { if !offset.is_null() { *offset = (*parser) .m_eventPtr - .wrapping_offset_from((*parser).m_buffer) as c_int + .wrapping_offset_from((*parser).m_buffer.as_ptr()) as c_int } if !size.is_null() { - *size = (*parser) - .m_bufferEnd - .wrapping_offset_from((*parser).m_buffer) as c_int + *size = (*parser).m_bufferEnd.try_into().unwrap(); } - return (*parser).m_buffer; + return (*parser).m_buffer.as_ptr(); } /* defined XML_CONTEXT_BYTES */ ptr::null() @@ -3150,15 +3114,16 @@ pub unsafe extern "C" fn XML_GetCurrentLineNumber(mut parser: XML_Parser) -> XML if parser.is_null() { return 0; } - if !(*parser).m_eventPtr.is_null() && (*parser).m_eventPtr >= (*parser).m_positionPtr { + let positionPtr = &(*parser).m_buffer[(*parser).m_positionIdx]; + if !(*parser).m_eventPtr.is_null() && (*parser).m_eventPtr >= positionPtr { (*(*parser).m_encoding).updatePosition( ExpatBufRef::new( - (*parser).m_positionPtr, + positionPtr, (*parser).m_eventPtr, ), &mut (*parser).m_position, ); - (*parser).m_positionPtr = (*parser).m_eventPtr + (*parser).m_positionIdx = (*parser).m_eventPtr.wrapping_offset_from((*parser).m_buffer.as_ptr()) as usize; } (*parser).m_position.lineNumber.wrapping_add(1) } @@ -3167,15 +3132,16 @@ pub unsafe extern "C" fn XML_GetCurrentColumnNumber(mut parser: XML_Parser) -> X if parser.is_null() { return 0; } - if !(*parser).m_eventPtr.is_null() && (*parser).m_eventPtr >= (*parser).m_positionPtr { + let positionPtr = &(*parser).m_buffer[(*parser).m_positionIdx]; + if !(*parser).m_eventPtr.is_null() && (*parser).m_eventPtr >= positionPtr { (*(*parser).m_encoding).updatePosition( ExpatBufRef::new( - (*parser).m_positionPtr, + positionPtr, (*parser).m_eventPtr, ), &mut (*parser).m_position, ); - (*parser).m_positionPtr = (*parser).m_eventPtr + (*parser).m_positionIdx = (*parser).m_eventPtr.wrapping_offset_from((*parser).m_buffer.as_ptr()) as usize; } (*parser).m_position.columnNumber } diff --git a/src/tests/runtests.rs b/src/tests/runtests.rs index 7b7c6d7f..0c63cf21 100644 --- a/src/tests/runtests.rs +++ b/src/tests/runtests.rs @@ -20394,7 +20394,10 @@ unsafe extern "C" fn test_nsalloc_realloc_long_prefix() { b"\x00".as_ptr() as *const c_char; let mut i: c_int = 0; - let max_realloc_count: c_int = 12; + + // REXPAT: max_realloc_count was 12. We realloc the main buffer rather than allocing, so + // that adds two reallocs to the parse. + let max_realloc_count: c_int = 13; i = 0; while i < max_realloc_count { reallocation_count = i as intptr_t; @@ -20442,7 +20445,10 @@ unsafe extern "C" fn test_nsalloc_realloc_longer_prefix() { b"\x00".as_ptr() as *const c_char; let mut i: c_int = 0; - let max_realloc_count: c_int = 12; + + // REXPAT: max_realloc_count was 12. We realloc the main buffer rather than allocing, so + // that adds two reallocs to the parse. + let max_realloc_count: c_int = 13; i = 0; while i < max_realloc_count { reallocation_count = i as intptr_t; From 8a8d77f88b6719eb5e3720b2ee5bfb4eafa9ed12 Mon Sep 17 00:00:00 2001 From: Stephen Crane Date: Fri, 7 Feb 2020 14:23:03 -0800 Subject: [PATCH 02/14] Fix OOB index with m_positionIdx --- src/lib/xmlparse.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/xmlparse.rs b/src/lib/xmlparse.rs index 7387f5ee..9b7e9e59 100644 --- a/src/lib/xmlparse.rs +++ b/src/lib/xmlparse.rs @@ -3114,7 +3114,7 @@ pub unsafe extern "C" fn XML_GetCurrentLineNumber(mut parser: XML_Parser) -> XML if parser.is_null() { return 0; } - let positionPtr = &(*parser).m_buffer[(*parser).m_positionIdx]; + let positionPtr = (*parser).m_buffer.as_ptr().wrapping_add((*parser).m_positionIdx); if !(*parser).m_eventPtr.is_null() && (*parser).m_eventPtr >= positionPtr { (*(*parser).m_encoding).updatePosition( ExpatBufRef::new( @@ -3132,7 +3132,7 @@ pub unsafe extern "C" fn XML_GetCurrentColumnNumber(mut parser: XML_Parser) -> X if parser.is_null() { return 0; } - let positionPtr = &(*parser).m_buffer[(*parser).m_positionIdx]; + let positionPtr = (*parser).m_buffer.as_ptr().wrapping_add((*parser).m_positionIdx); if !(*parser).m_eventPtr.is_null() && (*parser).m_eventPtr >= positionPtr { (*(*parser).m_encoding).updatePosition( ExpatBufRef::new( From e4d31aa71fa13797834b95efb9bc1e2c39ff4a01 Mon Sep 17 00:00:00 2001 From: Stephen Crane Date: Fri, 7 Feb 2020 14:23:26 -0800 Subject: [PATCH 03/14] Make parseEndPtr field an index instead of a pointer --- src/lib/xmlparse.rs | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/lib/xmlparse.rs b/src/lib/xmlparse.rs index 9b7e9e59..3e5bcd20 100644 --- a/src/lib/xmlparse.rs +++ b/src/lib/xmlparse.rs @@ -775,8 +775,8 @@ pub struct XML_ParserStruct { m_bufferStart: usize, // index after last character to be parsed m_bufferEnd: usize, - pub m_parseEndByteIndex: XML_Index, - pub m_parseEndPtr: *const c_char, + m_parseEndByteIndex: usize, + m_parseEndIdx: usize, pub m_dataBuf: *mut XML_Char, // Box<[XML_Char; INIT_DATA_BUF_SIZE]> pub m_dataBufEnd: *mut XML_Char, @@ -843,6 +843,17 @@ impl XML_ParserStruct { EncodingType::Internal => self.m_internalEncoding, } } + + // TODO(SJC): add a better err type + fn buffer_index(&self, p: *const c_char) -> Result { + if p < self.m_buffer.as_ptr() + || p >= self.m_buffer.as_ptr().wrapping_add(self.m_buffer.len()) + { + Err(()) + } else { + Ok(p.wrapping_offset_from(self.m_buffer.as_ptr()) as usize) + } + } } #[repr(C)] @@ -1629,7 +1640,7 @@ impl XML_ParserStruct { // index after last character to be parsed m_bufferEnd: 0, m_parseEndByteIndex: 0, - m_parseEndPtr: ptr::null(), + m_parseEndIdx: 0, m_dataBuf: ptr::null_mut(), // Box<[XML_Char; INIT_DATA_BUF_SIZE]> m_dataBufEnd: ptr::null_mut(), @@ -1768,8 +1779,8 @@ impl XML_ParserStruct { self.m_handlers.m_externalEntityRefHandlerArg = self as XML_Parser; self.m_bufferStart = 0; self.m_bufferEnd = 0; - self.m_parseEndByteIndex = 0i64; - self.m_parseEndPtr = ptr::null(); + self.m_parseEndByteIndex = 0; + self.m_parseEndIdx = 0; self.m_declElementType = ptr::null_mut(); self.m_declAttributeId = ptr::null_mut(); self.m_declEntity = ptr::null_mut(); @@ -2610,7 +2621,7 @@ impl XML_ParserStruct { return XML_Status::OK; } self.m_positionIdx = self.m_bufferStart; - self.m_parseEndPtr = self.m_buffer.as_ptr().add(self.m_bufferEnd); + self.m_parseEndIdx = self.m_bufferEnd; /* If data are left over from last buffer, and we now know that these data are the final chunk of input, then we have to check them again to detect errors based on that fact. @@ -2711,9 +2722,8 @@ impl XML_ParserStruct { start = self.m_buffer.as_ptr().add(self.m_bufferStart); self.m_positionIdx = self.m_bufferStart; self.m_bufferEnd += len as usize; - // TODO(SJC): is signed overflow an issue here? - self.m_parseEndPtr = self.m_buffer.as_ptr().add(self.m_bufferEnd); - self.m_parseEndByteIndex += len as c_long; + self.m_parseEndIdx = self.m_bufferEnd; + self.m_parseEndByteIndex += len as usize; self.m_parsingStatus.finalBuffer = isFinal != 0; self.m_errorCode = self.m_processor.expect("non-null function pointer")( self, @@ -2951,10 +2961,7 @@ impl XML_ParserStruct { self.m_parsingStatus.parsing = XML_Parsing::PARSING; self.m_errorCode = self.m_processor.expect("non-null function pointer")( self, - ExpatBufRef::new( - &self.m_buffer[self.m_bufferStart], - self.m_parseEndPtr, - ), + self.m_buffer[self.m_bufferStart..self.m_parseEndIdx].into(), &mut (&self.m_buffer[self.m_bufferStart] as *const _), ); if self.m_errorCode != XML_Error::NONE { @@ -3031,13 +3038,10 @@ pub unsafe extern "C" fn XML_GetCurrentByteIndex(mut parser: XML_Parser) -> XML_ return -1; } if !(*parser).m_eventPtr.is_null() { - return (*parser).m_parseEndByteIndex - - (*parser) - .m_parseEndPtr - .wrapping_offset_from((*parser).m_eventPtr) as c_long; + return ((*parser).m_parseEndByteIndex - ((*parser).m_parseEndIdx - (*parser).buffer_index((*parser).m_eventPtr).unwrap())) as XML_Index; } if cfg!(feature = "mozilla") { - return (*parser).m_parseEndByteIndex; + return (*parser).m_parseEndByteIndex as XML_Index; } -1 } From 0420f4525ac9e1fc7b2a6932ec161065c416186f Mon Sep 17 00:00:00 2001 From: Stephen Crane Date: Fri, 7 Feb 2020 14:30:01 -0800 Subject: [PATCH 04/14] Improve comments on fields --- src/lib/xmlparse.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/lib/xmlparse.rs b/src/lib/xmlparse.rs index 3e5bcd20..e952f5ae 100644 --- a/src/lib/xmlparse.rs +++ b/src/lib/xmlparse.rs @@ -771,11 +771,14 @@ pub struct XML_ParserStruct { macro works. */ pub m_userData: *mut c_void, m_buffer: Vec, - // index of first character to be parsed + // index in m_buffer of first character to be parsed m_bufferStart: usize, - // index after last character to be parsed + // index in m_buffer after last character to be parsed m_bufferEnd: usize, + // Absolute index after last character that has been parsed (in the overall + // input stream) m_parseEndByteIndex: usize, + // Index in m_buffer after last character that has been parsed m_parseEndIdx: usize, pub m_dataBuf: *mut XML_Char, // Box<[XML_Char; INIT_DATA_BUF_SIZE]> pub m_dataBufEnd: *mut XML_Char, From d6e2a2ddacdd12f1430045ad2c9eb9b2c9a8721d Mon Sep 17 00:00:00 2001 From: Stephen Crane Date: Fri, 7 Feb 2020 14:56:44 -0800 Subject: [PATCH 05/14] Make m_dataBuf an owned Box instead of a raw pointer --- src/lib/xmlparse.rs | 78 ++++++++++++++++++++++++--------------------- 1 file changed, 41 insertions(+), 37 deletions(-) diff --git a/src/lib/xmlparse.rs b/src/lib/xmlparse.rs index e952f5ae..2825b716 100644 --- a/src/lib/xmlparse.rs +++ b/src/lib/xmlparse.rs @@ -66,6 +66,7 @@ pub use ::libc::INT_MAX; use libc::{c_char, c_int, c_long, c_uint, c_ulong, c_ushort, c_void, size_t, memcpy, memcmp, memmove, memset}; use num_traits::{ToPrimitive,FromPrimitive}; +use std::collections::TryReserveError; use fallible_collections::FallibleBox; use std::alloc::{self, Layout}; @@ -780,8 +781,8 @@ pub struct XML_ParserStruct { m_parseEndByteIndex: usize, // Index in m_buffer after last character that has been parsed m_parseEndIdx: usize, - pub m_dataBuf: *mut XML_Char, // Box<[XML_Char; INIT_DATA_BUF_SIZE]> - pub m_dataBufEnd: *mut XML_Char, + // Temporary scratch buffer + m_dataBuf: Box<[XML_Char; INIT_DATA_BUF_SIZE as usize]>, // Handlers should be trait, with native C callback instance m_handlers: CXmlHandlers, @@ -1633,8 +1634,14 @@ pub unsafe extern "C" fn XML_ParserCreate_MM( XML_ParserStruct::create(encodingName, nameSep, dtd) } +<<<<<<< HEAD impl XML_ParserStruct { fn try_new(use_namespaces: bool, dtd: Rc) -> Result { +======= +impl<'scf> XML_ParserStruct<'scf> { + fn new(use_namespaces: bool) -> Result { + let m_dataBuf = Box::try_new([0; INIT_DATA_BUF_SIZE as usize])?; +>>>>>>> Make m_dataBuf an owned Box instead of a raw pointer Ok(Self { m_userData: ptr::null_mut(), m_buffer: Vec::new(), @@ -1644,8 +1651,7 @@ impl XML_ParserStruct { m_bufferEnd: 0, m_parseEndByteIndex: 0, m_parseEndIdx: 0, - m_dataBuf: ptr::null_mut(), // Box<[XML_Char; INIT_DATA_BUF_SIZE]> - m_dataBufEnd: ptr::null_mut(), + m_dataBuf, m_handlers: Default::default(), @@ -1724,7 +1730,12 @@ impl XML_ParserStruct { mut dtd: Rc, ) -> XML_Parser { let use_namespaces = !nameSep.is_null(); +<<<<<<< HEAD let mut parser = match Box::try_new(parser) { +======= + + let mut parser = match XML_ParserStruct::new(use_namespaces).and_then(Box::try_new) { +>>>>>>> Make m_dataBuf an owned Box instead of a raw pointer Ok(p) => p, Err(_) => return ptr::null_mut(), }; @@ -2086,10 +2097,16 @@ impl Drop for XML_ParserStruct { parser->m_dtd with the root parser, so we must not destroy it */ FREE!(self.m_groupConnector); +<<<<<<< HEAD FREE!(self.m_dataBuf); if self.m_handlers.m_unknownEncodingRelease.is_some() { self.m_handlers.m_unknownEncodingRelease .expect("non-null function pointer")(self.m_handlers.m_unknownEncodingData); +======= + if self.m_unknownEncodingRelease.is_some() { + self.m_unknownEncodingRelease + .expect("non-null function pointer")(self.m_unknownEncodingData); +>>>>>>> Make m_dataBuf an owned Box instead of a raw pointer } } } @@ -2733,11 +2750,8 @@ impl XML_ParserStruct { self.m_buffer[self.m_bufferStart..self.m_bufferEnd].into(), &mut start, ); -<<<<<<< HEAD -======= self.m_bufferStart = start.wrapping_offset_from(self.m_buffer.as_ptr()) as usize; ->>>>>>> Convert parser m_buffer field to a Rust managed Vec if self.m_errorCode != XML_Error::NONE { self.m_eventEndPtr = self.m_eventPtr; self.m_processor = Some(errorProcessor as Processor); @@ -2779,13 +2793,9 @@ pub unsafe extern "C" fn XML_ParseBuffer( (*parser).parseBuffer(len, isFinal) } -<<<<<<< HEAD -impl XML_ParserStruct { - pub unsafe fn getBuffer(&mut self, len: c_int) -> *mut c_void { -======= + impl <'scf> XML_ParserStruct<'scf> { pub unsafe fn getBuffer(&mut self, len: c_int) -> Option<&mut [c_char]> { ->>>>>>> Convert parser m_buffer field to a Rust managed Vec if len < 0 { self.m_errorCode = XML_Error::NO_MEMORY; return None; @@ -3595,12 +3605,13 @@ unsafe extern "C" fn externalEntityContentProcessor( result } -impl XML_ParserStruct { - unsafe fn doContent( - &mut self, + +impl<'scf> XML_ParserStruct<'scf> { + unsafe fn doContent<'a, 'b: 'a>( + &'b mut self, startTagLevel: c_int, enc_type: EncodingType, - mut buf: ExpatBufRef, + mut buf: ExpatBufRef<'a>, nextPtr: *mut *const c_char, haveMore: XML_Bool, ) -> XML_Error { @@ -4123,14 +4134,12 @@ impl XML_ParserStruct { } if self.m_handlers.hasCharacterData() { if MUST_CONVERT!(enc, buf.as_ptr()) { - let mut dataPtr = ExpatBufRefMut::new( - self.m_dataBuf as *mut ICHAR, - self.m_dataBufEnd as *mut ICHAR, - ); + let dataStart = self.m_dataBuf.as_ptr(); + let mut dataPtr = (&mut self.m_dataBuf[..]).into(); XmlConvert!(enc, &mut buf, &mut dataPtr); self.m_handlers.characterData( &ExpatBufRef::new( - self.m_dataBuf, + dataStart, dataPtr.as_ptr(), ), ); @@ -4160,10 +4169,8 @@ impl XML_ParserStruct { if MUST_CONVERT!(enc, buf.as_ptr()) { loop { let mut from_buf = buf.with_end(next); - let mut to_buf = ExpatBufRefMut::new( - self.m_dataBuf as *mut ICHAR, - self.m_dataBufEnd as *mut ICHAR, - ); + let dataStart = self.m_dataBuf.as_ptr(); + let mut to_buf = (&mut self.m_dataBuf[..]).into(); let convert_res_0: super::xmltok::XML_Convert_Result = XmlConvert!( enc, &mut from_buf, @@ -4171,7 +4178,10 @@ impl XML_ParserStruct { ); buf = buf.with_start(from_buf.as_ptr()); *eventEndPP = buf.as_ptr(); - let data_buf = ExpatBufRef::new(self.m_dataBuf, to_buf.as_ptr()); + let data_buf = ExpatBufRef::new( + dataStart, + to_buf.as_ptr(), + ); handlers.characterData(&data_buf); if convert_res_0 == super::xmltok::XML_Convert_Result::COMPLETED || convert_res_0 == super::xmltok::XML_Convert_Result::INPUT_INCOMPLETE @@ -4997,10 +5007,7 @@ unsafe extern "C" fn doCdataSection( if MUST_CONVERT!(enc, buf.as_ptr()) { loop { let mut from_buf = buf.with_end(next); - let mut to_buf = ExpatBufRefMut::new( - (*parser).m_dataBuf as *mut ICHAR, - (*parser).m_dataBufEnd as *mut ICHAR, - ); + let mut to_buf = (&mut (*parser).m_dataBuf[..]).into(); let convert_res: super::xmltok::XML_Convert_Result = XmlConvert!( enc, &mut from_buf, @@ -5010,7 +5017,7 @@ unsafe extern "C" fn doCdataSection( *eventEndPP = next; handlers.characterData( &ExpatBufRef::new( - (*parser).m_dataBuf, + (*parser).m_dataBuf.as_ptr(), to_buf.as_ptr(), ), ); @@ -7731,16 +7738,13 @@ unsafe extern "C" fn reportDefault( eventEndPP = &mut (*parser).m_eventEndPtr } loop { - let mut data_buf = ExpatBufRefMut::new( - (*parser).m_dataBuf as *mut ICHAR, - (*parser).m_dataBufEnd as *mut ICHAR, - ); + let mut data_buf = (&mut (*parser).m_dataBuf[..]).into(); convert_res = XmlConvert!(enc, &mut buf, &mut data_buf); *eventEndPP = buf.as_ptr(); let defaultRan = (*parser).m_handlers.default( - (*parser).m_dataBuf, - data_buf.as_ptr().wrapping_offset_from((*parser).m_dataBuf).try_into().unwrap(), + (*parser).m_dataBuf.as_ptr(), + data_buf.as_ptr().wrapping_offset_from((*parser).m_dataBuf.as_ptr()).try_into().unwrap(), ); // Previously unwrapped an Option From df2f40cbb9d8d64ddd1322cae19311833c85e9f8 Mon Sep 17 00:00:00 2001 From: Stephen Crane Date: Fri, 7 Feb 2020 15:36:39 -0800 Subject: [PATCH 06/14] Combine XML_ParserStruct::new and XML_ParserStruct::create --- src/lib/xmlparse.rs | 70 ++++++++++++++++----------------------------- 1 file changed, 25 insertions(+), 45 deletions(-) diff --git a/src/lib/xmlparse.rs b/src/lib/xmlparse.rs index 2825b716..46be6527 100644 --- a/src/lib/xmlparse.rs +++ b/src/lib/xmlparse.rs @@ -66,7 +66,6 @@ pub use ::libc::INT_MAX; use libc::{c_char, c_int, c_long, c_uint, c_ulong, c_ushort, c_void, size_t, memcpy, memcmp, memmove, memset}; use num_traits::{ToPrimitive,FromPrimitive}; -use std::collections::TryReserveError; use fallible_collections::FallibleBox; use std::alloc::{self, Layout}; @@ -1634,15 +1633,22 @@ pub unsafe extern "C" fn XML_ParserCreate_MM( XML_ParserStruct::create(encodingName, nameSep, dtd) } -<<<<<<< HEAD -impl XML_ParserStruct { - fn try_new(use_namespaces: bool, dtd: Rc) -> Result { -======= + // fn try_new(use_namespaces: bool, dtd: Rc) -> Result { + // fn new(use_namespaces: bool) -> Result { impl<'scf> XML_ParserStruct<'scf> { - fn new(use_namespaces: bool) -> Result { - let m_dataBuf = Box::try_new([0; INIT_DATA_BUF_SIZE as usize])?; ->>>>>>> Make m_dataBuf an owned Box instead of a raw pointer - Ok(Self { + unsafe fn create( + mut encodingName: *const XML_Char, + mut nameSep: *const XML_Char, + mut dtd: *mut DTD<'scf>, + ) -> XML_Parser { + let use_namespaces = !nameSep.is_null(); + + let m_dataBuf = match Box::try_new([0; INIT_DATA_BUF_SIZE as usize]) { + Ok(b) => b, + Err(_) => return ptr::null_mut(), + }; + + let parser = Self { m_userData: ptr::null_mut(), m_buffer: Vec::new(), // index of first character to be parsed @@ -1712,8 +1718,9 @@ impl<'scf> XML_ParserStruct<'scf> { m_temp2Pool: StringPool::try_new()?, m_groupConnector: ptr::null_mut(), m_groupSize: 0, - m_namespaceSeparator: 0, - is_child_parser: false, + // is_child_parser: false, + m_namespaceSeparator: ASCII_EXCL as XML_Char, + // m_parentParser: ptr::null_mut(), m_parsingStatus: XML_ParsingStatus::default(), m_isParamEntity: false, m_useForeignDTD: false, @@ -1721,21 +1728,9 @@ impl<'scf> XML_ParserStruct<'scf> { #[cfg(feature = "mozilla")] m_mismatch: ptr::null(), - }) - } + }; - unsafe fn create( - mut encodingName: *const XML_Char, - mut nameSep: *const XML_Char, - mut dtd: Rc, - ) -> XML_Parser { - let use_namespaces = !nameSep.is_null(); -<<<<<<< HEAD let mut parser = match Box::try_new(parser) { -======= - - let mut parser = match XML_ParserStruct::new(use_namespaces).and_then(Box::try_new) { ->>>>>>> Make m_dataBuf an owned Box instead of a raw pointer Ok(p) => p, Err(_) => return ptr::null_mut(), }; @@ -1747,22 +1742,13 @@ impl<'scf> XML_ParserStruct<'scf> { if parser.typed_atts.try_reserve(INIT_ATTS_SIZE as usize).is_err() { return ptr::null_mut(); } - parser.m_dataBuf = MALLOC![XML_Char; INIT_DATA_BUF_SIZE]; - if parser.m_dataBuf.is_null() { + + parser.m_dtd = if !dtd.is_null() { dtd } else { dtdCreate() }; + if parser.m_dtd.is_null() { return ptr::null_mut(); } - parser.m_dataBufEnd = parser.m_dataBuf.offset(INIT_DATA_BUF_SIZE as isize); - parser.m_freeBindingList = ptr::null_mut(); - parser.m_freeTagList = None; - parser.m_freeInternalEntities = ptr::null_mut(); - parser.m_groupSize = 0; - parser.m_groupConnector = ptr::null_mut(); - parser.m_initEncoding = None; - parser.m_handlers.m_unknownEncoding = None; - parser.m_namespaceSeparator = ASCII_EXCL as XML_Char; - parser.m_ns = false; - parser.m_ns_triplets = false; - parser.m_protocolEncodingName = ptr::null(); + parser.m_tempPool.init(); + parser.m_temp2Pool.init(); parser.init(encodingName); if !encodingName.is_null() && parser.m_protocolEncodingName.is_null() { return ptr::null_mut(); @@ -1776,6 +1762,7 @@ impl<'scf> XML_ParserStruct<'scf> { { parser.m_mismatch = ptr::null(); } + Box::into_raw(parser) } @@ -2097,16 +2084,9 @@ impl Drop for XML_ParserStruct { parser->m_dtd with the root parser, so we must not destroy it */ FREE!(self.m_groupConnector); -<<<<<<< HEAD - FREE!(self.m_dataBuf); if self.m_handlers.m_unknownEncodingRelease.is_some() { self.m_handlers.m_unknownEncodingRelease .expect("non-null function pointer")(self.m_handlers.m_unknownEncodingData); -======= - if self.m_unknownEncodingRelease.is_some() { - self.m_unknownEncodingRelease - .expect("non-null function pointer")(self.m_unknownEncodingData); ->>>>>>> Make m_dataBuf an owned Box instead of a raw pointer } } } From 86ee1c1ce97a7aea428d35ac3dbf05aae0215687 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Fri, 31 Jul 2020 16:59:19 -0700 Subject: [PATCH 07/14] Add comment to address review feedback --- src/lib/xmlparse.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/xmlparse.rs b/src/lib/xmlparse.rs index 46be6527..3dede18c 100644 --- a/src/lib/xmlparse.rs +++ b/src/lib/xmlparse.rs @@ -2718,6 +2718,7 @@ impl XML_ParserStruct { } /* fall through */ self.m_parsingStatus.parsing = XML_Parsing::PARSING; + // convert in-out parameter `start` from index to pointer // TODO(SJC): is signed overflow an issue here? start = self.m_buffer.as_ptr().add(self.m_bufferStart); self.m_positionIdx = self.m_bufferStart; @@ -2730,7 +2731,7 @@ impl XML_ParserStruct { self.m_buffer[self.m_bufferStart..self.m_bufferEnd].into(), &mut start, ); - + // convert in-out parameter `start` from pointer back to index self.m_bufferStart = start.wrapping_offset_from(self.m_buffer.as_ptr()) as usize; if self.m_errorCode != XML_Error::NONE { self.m_eventEndPtr = self.m_eventPtr; From 3bbabe70e82d5744399375f87acb32adb6c09c78 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Fri, 31 Jul 2020 22:17:48 -0700 Subject: [PATCH 08/14] Convert m_positionIdx into Option --- src/lib/xmlparse.rs | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/lib/xmlparse.rs b/src/lib/xmlparse.rs index 3dede18c..943e30b1 100644 --- a/src/lib/xmlparse.rs +++ b/src/lib/xmlparse.rs @@ -796,7 +796,7 @@ pub struct XML_ParserStruct { pub m_errorCode: XML_Error, pub m_eventPtr: *const c_char, pub m_eventEndPtr: *const c_char, - pub m_positionIdx: usize, + pub m_positionIdx: Option, pub m_openInternalEntities: *mut OpenInternalEntity, pub m_freeInternalEntities: *mut OpenInternalEntity, pub m_defaultExpandInternalEntities: XML_Bool, @@ -1686,7 +1686,7 @@ impl<'scf> XML_ParserStruct<'scf> { m_errorCode: XML_Error::NONE, m_eventPtr: ptr::null(), m_eventEndPtr: ptr::null(), - m_positionIdx: 0, + m_positionIdx: None, m_openInternalEntities: ptr::null_mut(), m_freeInternalEntities: ptr::null_mut(), m_defaultExpandInternalEntities: false, @@ -1801,7 +1801,7 @@ impl<'scf> XML_ParserStruct<'scf> { self.m_errorCode = XML_Error::NONE; self.m_eventPtr = ptr::null(); self.m_eventEndPtr = ptr::null(); - self.m_positionIdx = 0; + self.m_positionIdx = Some(0); self.m_openInternalEntities = ptr::null_mut(); self.m_defaultExpandInternalEntities = true; self.m_tagLevel = 0; @@ -2620,7 +2620,7 @@ impl XML_ParserStruct { if isFinal == 0 { return XML_Status::OK; } - self.m_positionIdx = self.m_bufferStart; + self.m_positionIdx = Some(self.m_bufferStart); self.m_parseEndIdx = self.m_bufferEnd; /* If data are left over from last buffer, and we now know that these data are the final chunk of input, then we have to check them again @@ -2649,10 +2649,10 @@ impl XML_ParserStruct { * LCOV_EXCL_START */ (*self.m_encoding).updatePosition( - self.m_buffer[self.m_positionIdx..self.m_bufferStart].into(), + self.m_buffer[self.m_positionIdx.unwrap()..self.m_bufferStart].into(), &mut self.m_position, ); - self.m_positionIdx = self.m_bufferStart; + self.m_positionIdx = Some(self.m_bufferStart); return XML_Status::SUSPENDED; } XML_Parsing::INITIALIZED | XML_Parsing::PARSING => { @@ -2721,7 +2721,7 @@ impl XML_ParserStruct { // convert in-out parameter `start` from index to pointer // TODO(SJC): is signed overflow an issue here? start = self.m_buffer.as_ptr().add(self.m_bufferStart); - self.m_positionIdx = self.m_bufferStart; + self.m_positionIdx = Some(self.m_bufferStart); self.m_bufferEnd += len as usize; self.m_parseEndIdx = self.m_bufferEnd; self.m_parseEndByteIndex += len as usize; @@ -2753,10 +2753,10 @@ impl XML_ParserStruct { } } (*self.m_encoding).updatePosition( - self.m_buffer[self.m_positionIdx..self.m_bufferStart].into(), + self.m_buffer[self.m_positionIdx.unwrap()..self.m_bufferStart].into(), &mut self.m_position, ); - self.m_positionIdx = self.m_bufferStart; + self.m_positionIdx = Some(self.m_bufferStart); result } } @@ -2846,7 +2846,7 @@ impl <'scf> XML_ParserStruct<'scf> { } self.m_eventEndPtr = ptr::null(); self.m_eventPtr = ptr::null(); - self.m_positionIdx = 0; + self.m_positionIdx = None; } Some(&mut self.m_buffer[self.m_bufferEnd..]) } @@ -2975,10 +2975,10 @@ impl XML_ParserStruct { } } (*self.m_encoding).updatePosition( - self.m_buffer[self.m_positionIdx..self.m_bufferStart].into(), + self.m_buffer[self.m_positionIdx.unwrap()..self.m_bufferStart].into(), &mut self.m_position, ); - self.m_positionIdx = self.m_bufferStart; + self.m_positionIdx = Some(self.m_bufferStart); #[cfg(feature = "mozilla")] { @@ -3112,7 +3112,7 @@ pub unsafe extern "C" fn XML_GetCurrentLineNumber(mut parser: XML_Parser) -> XML if parser.is_null() { return 0; } - let positionPtr = (*parser).m_buffer.as_ptr().wrapping_add((*parser).m_positionIdx); + let positionPtr = (*parser).m_buffer.as_ptr().wrapping_add((*parser).m_positionIdx.unwrap()); if !(*parser).m_eventPtr.is_null() && (*parser).m_eventPtr >= positionPtr { (*(*parser).m_encoding).updatePosition( ExpatBufRef::new( @@ -3121,7 +3121,9 @@ pub unsafe extern "C" fn XML_GetCurrentLineNumber(mut parser: XML_Parser) -> XML ), &mut (*parser).m_position, ); - (*parser).m_positionIdx = (*parser).m_eventPtr.wrapping_offset_from((*parser).m_buffer.as_ptr()) as usize; + (*parser).m_positionIdx = Some( + (*parser).m_eventPtr.wrapping_offset_from((*parser).m_buffer.as_ptr()) as usize + ); } (*parser).m_position.lineNumber.wrapping_add(1) } @@ -3130,7 +3132,7 @@ pub unsafe extern "C" fn XML_GetCurrentColumnNumber(mut parser: XML_Parser) -> X if parser.is_null() { return 0; } - let positionPtr = (*parser).m_buffer.as_ptr().wrapping_add((*parser).m_positionIdx); + let positionPtr = (*parser).m_buffer.as_ptr().wrapping_add((*parser).m_positionIdx.unwrap()); if !(*parser).m_eventPtr.is_null() && (*parser).m_eventPtr >= positionPtr { (*(*parser).m_encoding).updatePosition( ExpatBufRef::new( @@ -3139,7 +3141,9 @@ pub unsafe extern "C" fn XML_GetCurrentColumnNumber(mut parser: XML_Parser) -> X ), &mut (*parser).m_position, ); - (*parser).m_positionIdx = (*parser).m_eventPtr.wrapping_offset_from((*parser).m_buffer.as_ptr()) as usize; + (*parser).m_positionIdx = Some( + (*parser).m_eventPtr.wrapping_offset_from((*parser).m_buffer.as_ptr()) as usize + ); } (*parser).m_position.columnNumber } From ac1bf73c0865089cdc1adc5d36b7448a9779d21e Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Sat, 1 Aug 2020 03:23:54 -0700 Subject: [PATCH 09/14] Rebase onto master --- src/expat_h.rs | 2 +- src/lib/xmlparse.rs | 90 +++++++++++++++++++++---------------------- src/tests/runtests.rs | 4 +- src/xmlwf/xmlfile.rs | 4 +- 4 files changed, 50 insertions(+), 50 deletions(-) diff --git a/src/expat_h.rs b/src/expat_h.rs index 948b1f98..7fad21fe 100644 --- a/src/expat_h.rs +++ b/src/expat_h.rs @@ -38,7 +38,7 @@ use num_derive::FromPrimitive; use num_derive::ToPrimitive; use num_traits::ToPrimitive; -pub type XML_Parser = *mut XML_ParserStruct; +pub type XML_Parser<'scf> = *mut XML_ParserStruct<'scf>; pub type XML_Bool = bool; /* The XML_Status enum gives the possible return values for several API functions. The preprocessor #defines are included so this diff --git a/src/lib/xmlparse.rs b/src/lib/xmlparse.rs index 943e30b1..7382dd45 100644 --- a/src/lib/xmlparse.rs +++ b/src/lib/xmlparse.rs @@ -289,7 +289,7 @@ trait XmlHandlers { #[repr(C)] #[derive(Clone)] -struct CXmlHandlers { +struct CXmlHandlers<'scf> { m_attlistDeclHandler: XML_AttlistDeclHandler, m_characterDataHandler: XML_CharacterDataHandler, m_commentHandler: XML_CommentHandler, @@ -301,7 +301,7 @@ struct CXmlHandlers { m_endNamespaceDeclHandler: XML_EndNamespaceDeclHandler, m_entityDeclHandler: XML_EntityDeclHandler, m_externalEntityRefHandler: XML_ExternalEntityRefHandler, - m_externalEntityRefHandlerArg: XML_Parser, + m_externalEntityRefHandlerArg: XML_Parser<'scf>, m_handlerArg: *mut c_void, m_notationDeclHandler: XML_NotationDeclHandler, m_notStandaloneHandler: XML_NotStandaloneHandler, @@ -320,7 +320,7 @@ struct CXmlHandlers { m_xmlDeclHandler: XML_XmlDeclHandler, } -impl Default for CXmlHandlers { +impl<'scf> Default for CXmlHandlers<'scf> { fn default() -> Self { CXmlHandlers { m_attlistDeclHandler: None, @@ -370,7 +370,7 @@ impl EncodingType { } } -impl CXmlHandlers { +impl<'scf> CXmlHandlers<'scf> { fn setStartElement(&mut self, handler: XML_StartElementHandler) { self.m_startElementHandler = handler; } @@ -460,7 +460,7 @@ impl CXmlHandlers { } } -impl XmlHandlers for CXmlHandlers { +impl<'scf> XmlHandlers for CXmlHandlers<'scf> { unsafe fn startElement(&self, a: *const XML_Char, b: &mut [Attribute]) -> bool { self.m_startElementHandler.map(|handler| { handler(self.m_handlerArg, a, b.as_mut_ptr() as *mut *const XML_Char); @@ -766,7 +766,7 @@ impl Attribute { } #[repr(C)] -pub struct XML_ParserStruct { +pub struct XML_ParserStruct<'scf> { /* The first member must be m_userData so that the XML_GetUserData macro works. */ pub m_userData: *mut c_void, @@ -784,7 +784,7 @@ pub struct XML_ParserStruct { m_dataBuf: Box<[XML_Char; INIT_DATA_BUF_SIZE as usize]>, // Handlers should be trait, with native C callback instance - m_handlers: CXmlHandlers, + m_handlers: CXmlHandlers<'scf>, pub m_encoding: *const ENCODING, pub m_initEncoding: Option, pub m_internalEncoding: &'static super::xmltok::ENCODING, @@ -839,7 +839,7 @@ pub struct XML_ParserStruct { pub m_mismatch: *const XML_Char, } -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { fn encoding<'a, 'b>(&'a self, enc_type: EncodingType) -> &'b dyn XmlEncoding { match enc_type { EncodingType::Normal => unsafe { &*self.m_encoding }, @@ -1550,9 +1550,9 @@ impl Drop for Tag { external protocol or NULL if there is none specified. */ #[no_mangle] -pub unsafe extern "C" fn XML_ParserCreate( +pub unsafe extern "C" fn XML_ParserCreate<'scf>( mut encodingName: *const XML_Char -) -> XML_Parser { +) -> XML_Parser<'scf> { XML_ParserCreate_MM( encodingName, None, @@ -1571,10 +1571,10 @@ pub unsafe extern "C" fn XML_ParserCreate( triplets (see XML_SetReturnNSTriplet). */ #[no_mangle] -pub unsafe extern "C" fn XML_ParserCreateNS( +pub unsafe extern "C" fn XML_ParserCreateNS<'scf>( mut encodingName: *const XML_Char, mut nsSep: XML_Char, -) -> XML_Parser { +) -> XML_Parser<'scf> { let mut tmp: [XML_Char; 2] = [0; 2]; tmp[0] = nsSep; XML_ParserCreate_MM( @@ -1594,7 +1594,7 @@ const implicitContext: [XML_Char; 41] = XML_STR![ /* To avoid warnings about unused functions: */ -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { /* only valid for root parser */ unsafe fn startParsing(&mut self) -> XML_Bool { /* hash functions must be initialized before setContext() is called */ @@ -1639,8 +1639,8 @@ impl<'scf> XML_ParserStruct<'scf> { unsafe fn create( mut encodingName: *const XML_Char, mut nameSep: *const XML_Char, - mut dtd: *mut DTD<'scf>, - ) -> XML_Parser { + mut dtd: Rc, + ) -> XML_Parser<'scf> { let use_namespaces = !nameSep.is_null(); let m_dataBuf = match Box::try_new([0; INIT_DATA_BUF_SIZE as usize]) { @@ -1714,11 +1714,17 @@ impl<'scf> XML_ParserStruct<'scf> { typed_atts: Vec::new(), m_nsAtts: HashSet::new(), m_position: super::xmltok::Position::default(), - m_tempPool: StringPool::try_new()?, - m_temp2Pool: StringPool::try_new()?, + m_tempPool: match StringPool::try_new() { + Ok(sp) => sp, + Err(_) => return ptr::null_mut(), + }, + m_temp2Pool: match StringPool::try_new() { + Ok(sp) => sp, + Err(_) => return ptr::null_mut(), + }, m_groupConnector: ptr::null_mut(), m_groupSize: 0, - // is_child_parser: false, + is_child_parser: false, m_namespaceSeparator: ASCII_EXCL as XML_Char, // m_parentParser: ptr::null_mut(), m_parsingStatus: XML_ParsingStatus::default(), @@ -1743,12 +1749,6 @@ impl<'scf> XML_ParserStruct<'scf> { return ptr::null_mut(); } - parser.m_dtd = if !dtd.is_null() { dtd } else { dtdCreate() }; - if parser.m_dtd.is_null() { - return ptr::null_mut(); - } - parser.m_tempPool.init(); - parser.m_temp2Pool.init(); parser.init(encodingName); if !encodingName.is_null() && parser.m_protocolEncodingName.is_null() { return ptr::null_mut(); @@ -1838,7 +1838,7 @@ impl<'scf> XML_ParserStruct<'scf> { Added in Expat 1.95.3. */ -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { pub unsafe fn reset(&mut self, encodingName: *const XML_Char) -> XML_Bool { let mut openEntityList: *mut OpenInternalEntity = ptr::null_mut(); if self.is_child_parser { @@ -1893,7 +1893,7 @@ pub unsafe extern "C" fn XML_ParserReset(parser: XML_Parser, encodingName: *cons has no effect and returns XML_Status::ERROR. */ -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { pub unsafe fn setEncoding(&mut self, encodingName: *const XML_Char) -> XML_Status { /* Block after XML_Parse()/XML_ParseBuffer() has been called. XXX There's no way for the caller to determine which of the @@ -1949,13 +1949,13 @@ pub unsafe extern "C" fn XML_SetEncoding( Otherwise returns a new XML_Parser object. */ #[no_mangle] -pub unsafe extern "C" fn XML_ExternalEntityParserCreate( - mut oldParser: Option<&XML_ParserStruct>, +pub unsafe extern "C" fn XML_ExternalEntityParserCreate<'scf>( + mut oldParser: Option<&'scf XML_ParserStruct<'scf>>, mut context: *const XML_Char, mut encodingName: *const XML_Char, -) -> XML_Parser { +) -> XML_Parser<'scf> { /* Validate the oldParser parameter before we pull everything out of it */ - let mut oldParser = match oldParser { + let mut oldParser: &'scf XML_ParserStruct<'scf> = match oldParser { Some(parser) => parser, None => return ptr::null_mut() }; @@ -1973,7 +1973,7 @@ pub unsafe extern "C" fn XML_ExternalEntityParserCreate( here. This makes this function more painful to follow than it would be otherwise. */ - let mut parser = if oldParser.m_ns { + let mut parser: XML_Parser<'scf> = if oldParser.m_ns { let mut tmp: [XML_Char; 2] = [0; 2]; *tmp.as_mut_ptr() = oldParser.m_namespaceSeparator; XML_ParserStruct::create(encodingName, tmp.as_mut_ptr(), newDtd) @@ -2057,7 +2057,7 @@ unsafe fn destroyBindings(mut bindings: *mut Binding) { } } -impl Drop for XML_ParserStruct { +impl<'scf> Drop for XML_ParserStruct<'scf> { /* Frees memory used by the parser. */ fn drop(&mut self) { let mut entityList: *mut OpenInternalEntity = ptr::null_mut(); @@ -2591,7 +2591,7 @@ pub unsafe extern "C" fn XML_SetHashSalt(_: XML_Parser, _: c_ulong) -> c_int { values. */ -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { pub unsafe fn parse(&mut self, s: *const c_char, len: c_int, isFinal: c_int) -> XML_Status { if len < 0 || s.is_null() && len != 0 { return XML_Status::ERROR; @@ -2695,7 +2695,7 @@ pub unsafe extern "C" fn XML_Parse( (*parser).parse(s, len, isFinal) } -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { pub unsafe fn parseBuffer(&mut self, len: c_int, isFinal: c_int) -> XML_Status { let mut start: *const c_char = ptr::null(); let mut result: XML_Status = XML_Status::OK; @@ -2896,7 +2896,7 @@ pub unsafe extern "C" fn XML_GetBuffer(mut parser: XML_Parser, mut len: c_int) - When suspended, parsing can be resumed by calling XML_ResumeParser(). */ -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { pub unsafe fn stopParser(&mut self, resumable: XML_Bool) -> XML_Status { match self.m_parsingStatus.parsing { XML_Parsing::SUSPENDED => { @@ -2945,7 +2945,7 @@ pub unsafe extern "C" fn XML_StopParser(parser: XML_Parser, resumable: XML_Bool) That is, the parent parser will not resume by itself and it is up to the application to call XML_ResumeParser() on it at the appropriate moment. */ -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { pub unsafe fn resumeParser(&mut self) -> XML_Status { let mut result: XML_Status = XML_Status::OK; if self.m_parsingStatus.parsing != XML_Parsing::SUSPENDED { @@ -3388,7 +3388,7 @@ pub unsafe extern "C" fn MOZ_XML_ProcessingEntityValue(parser: XML_Parser) -> XM processed, and not yet closed, we need to store tag->rawName in a more permanent location, since the parse buffer is about to be discarded. */ -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { unsafe fn storeRawNames(&mut self) -> XML_Bool { let mut tStk = &mut self.m_tagStack; while let Some(tag) = tStk { @@ -5177,7 +5177,7 @@ unsafe extern "C" fn doIgnoreSection( } /* XML_DTD */ -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { unsafe fn initializeEncoding(&mut self) -> XML_Error { let mut s: *const c_char = ptr::null(); if cfg!(feature = "unicode") { @@ -5343,7 +5343,7 @@ impl XML_ParserStruct { } } -impl CXmlHandlers { +impl<'scf> CXmlHandlers<'scf> { unsafe fn handleUnknownEncoding( &mut self, mut encodingName: *const XML_Char, @@ -5595,7 +5595,7 @@ unsafe extern "C" fn prologProcessor( ); } -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { unsafe fn doPrologHandleEntityRef( &mut self, mut entity: *mut Entity, @@ -6962,7 +6962,7 @@ unsafe extern "C" fn epilogProcessor( } } -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { unsafe fn processInternalEntity( &mut self, mut entity: *mut Entity, @@ -7788,7 +7788,7 @@ unsafe extern "C" fn defineAttribute( } } -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { unsafe fn setElementTypePrefix( &mut self, mut elementType: *mut ElementType, @@ -7926,7 +7926,7 @@ impl XML_ParserStruct { const CONTEXT_SEP: XML_Char = ASCII_FF as XML_Char; -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { unsafe fn getContext(&mut self) -> bool { let mut needSep = false; if !self.m_dtd.defaultPrefix.get().binding.is_null() { @@ -8034,7 +8034,7 @@ impl XML_ParserStruct { } } -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { unsafe fn setContext(&mut self, mut context: *const XML_Char) -> XML_Bool { let mut s: *const XML_Char = context; while *context != '\u{0}' as XML_Char { @@ -8241,7 +8241,7 @@ unsafe extern "C" fn keylen(mut s: KEY) -> size_t { } -impl XML_ParserStruct { +impl<'scf> XML_ParserStruct<'scf> { fn build_node<'a, 'b>( &self, src_node: usize, diff --git a/src/tests/runtests.rs b/src/tests/runtests.rs index 0c63cf21..44cd6c96 100644 --- a/src/tests/runtests.rs +++ b/src/tests/runtests.rs @@ -270,8 +270,8 @@ pub type DefaultCheck = default_check; #[repr(C)] #[derive(Copy, Clone)] -pub struct DataIssue240 { - pub parser: XML_Parser, +pub struct DataIssue240<'scf> { + pub parser: XML_Parser<'scf>, pub deep: c_int, } /* ptrdiff_t */ diff --git a/src/xmlwf/xmlfile.rs b/src/xmlwf/xmlfile.rs index fecd54ba..55354de9 100644 --- a/src/xmlwf/xmlfile.rs +++ b/src/xmlwf/xmlfile.rs @@ -22,8 +22,8 @@ pub use ::libc::{perror, fprintf, O_RDONLY}; #[repr(C)] #[derive(Copy, Clone)] -pub struct PROCESS_ARGS { - pub parser: XML_Parser, +pub struct PROCESS_ARGS<'scf> { + pub parser: XML_Parser<'scf>, pub retPtr: *mut c_int, } /* From c0c37cea0d033fc7acc030f2baf3ceb551c639f7 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Sun, 2 Aug 2020 02:24:15 -0700 Subject: [PATCH 10/14] Call self.parseBuffer(...) instead of XML_ParseBuffer(self,...) --- src/lib/xmlparse.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/xmlparse.rs b/src/lib/xmlparse.rs index 7382dd45..d2e958bd 100644 --- a/src/lib/xmlparse.rs +++ b/src/lib/xmlparse.rs @@ -2671,7 +2671,7 @@ impl<'scf> XML_ParserStruct<'scf> { /* not defined XML_CONTEXT_BYTES */ if let Some(buff) = self.getBuffer(len) { buff[..len as usize].copy_from_slice(std::slice::from_raw_parts(s, len as usize)); - XML_ParseBuffer(self, len, isFinal) + self.parseBuffer(len, isFinal) } else { XML_Status::ERROR } From 38c3986d41d3af2fa55d476fd13cfc3912b92b07 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Sun, 2 Aug 2020 23:00:24 -0700 Subject: [PATCH 11/14] Fix call to reserve capacity in getBuffer --- src/lib/xmlparse.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/xmlparse.rs b/src/lib/xmlparse.rs index d2e958bd..e3db7a59 100644 --- a/src/lib/xmlparse.rs +++ b/src/lib/xmlparse.rs @@ -2829,7 +2829,8 @@ impl <'scf> XML_ParserStruct<'scf> { } } } - if self.m_buffer.try_reserve(bufferSize as usize).is_err() { + let additional = bufferSize as usize - self.m_buffer.capacity(); + if self.m_buffer.try_reserve_exact(additional).is_err() { self.m_errorCode = XML_Error::NO_MEMORY; return None; } From 480ed7465365d8dd969e516e59760fb8d6ebd51f Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Sun, 2 Aug 2020 23:02:50 -0700 Subject: [PATCH 12/14] Revert many of testsuite changes from e9dd9c3 --- src/tests/runtests.rs | 42 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/src/tests/runtests.rs b/src/tests/runtests.rs index 44cd6c96..cdbcc94a 100644 --- a/src/tests/runtests.rs +++ b/src/tests/runtests.rs @@ -17700,14 +17700,13 @@ unsafe extern "C" fn test_alloc_realloc_subst_public_entity_value() { alloc_setup(); i += 1 } - // We no longer need to reallocate - if i > 0 && i < max_realloc_count { + if i == 0 { crate::minicheck::_fail_unless( 0i32, b"/home/sjcrane/projects/c2rust/libexpat/upstream/expat/tests/runtests.c\x00".as_ptr() as *const c_char, 8459i32, - b"Parsing required reallocation\x00".as_ptr() as *const c_char, + b"Parsing worked despite failing reallocation\x00".as_ptr() as *const c_char, ); } if i >= max_realloc_count { @@ -18102,14 +18101,13 @@ unsafe extern "C" fn test_alloc_realloc_attribute_enum_value() { alloc_setup(); i += 1 } - // We no longer need to reallocate - if i > 0 && i < max_realloc_count { + if i == 0 { crate::minicheck::_fail_unless( 0i32, b"/home/sjcrane/projects/c2rust/libexpat/upstream/expat/tests/runtests.c\x00".as_ptr() as *const c_char, 8689i32, - b"Parse required reallocation\x00".as_ptr() as *const c_char, + b"Parsing worked despite failing reallocator\x00".as_ptr() as *const c_char, ); } if i >= max_realloc_count { @@ -18171,14 +18169,13 @@ unsafe extern "C" fn test_alloc_realloc_implied_attribute() { alloc_setup(); i += 1 } - // We no longer need to reallocate - if i > 0 && i < max_realloc_count { + if i == 0 { crate::minicheck::_fail_unless( 0i32, b"/home/sjcrane/projects/c2rust/libexpat/upstream/expat/tests/runtests.c\x00".as_ptr() as *const c_char, 8739i32, - b"Parse required reallocation\x00".as_ptr() as *const c_char, + b"Parsing worked despite failing reallocation\x00".as_ptr() as *const c_char, ); } if i >= max_realloc_count { @@ -18240,14 +18237,13 @@ unsafe extern "C" fn test_alloc_realloc_default_attribute() { alloc_setup(); i += 1 } - // We no longer need to reallocate - if i > 0 && i < max_realloc_count { + if i == 0 { crate::minicheck::_fail_unless( 0i32, b"/home/sjcrane/projects/c2rust/libexpat/upstream/expat/tests/runtests.c\x00".as_ptr() as *const c_char, 8789i32, - b"Parse required reallocation\x00".as_ptr() as *const c_char, + b"Parse succeeded despite failing reallocator\x00".as_ptr() as *const c_char, ); } if i >= max_realloc_count { @@ -18962,14 +18958,13 @@ unsafe extern "C" fn test_alloc_realloc_long_attribute_value() { alloc_setup(); i += 1 } - // We no longer need to reallocate - if i > 0 && i < max_realloc_count { + if i == 0 { crate::minicheck::_fail_unless( 0i32, b"/home/sjcrane/projects/c2rust/libexpat/upstream/expat/tests/runtests.c\x00".as_ptr() as *const c_char, 9202i32, - b"Parse required reallocation\x00".as_ptr() as *const c_char, + b"Parse succeeded despite failing reallocator\x00".as_ptr() as *const c_char, ); } if i >= max_realloc_count { @@ -19277,14 +19272,13 @@ unsafe extern "C" fn test_alloc_realloc_param_entity_newline() { alloc_setup(); i += 1 } - // We no longer need to reallocate - if i > 0 && i < max_realloc_count { + if i == 0 { crate::minicheck::_fail_unless( 0i32, b"/home/sjcrane/projects/c2rust/libexpat/upstream/expat/tests/runtests.c\x00".as_ptr() as *const c_char, 9421i32, - b"Parse required reallocation\x00".as_ptr() as *const c_char, + b"Parse succeeded despite failing reallocator\x00".as_ptr() as *const c_char, ); } if i > max_realloc_count { @@ -19343,14 +19337,13 @@ unsafe extern "C" fn test_alloc_realloc_ce_extends_pe() { alloc_setup(); i += 1 } - // We no longer need to reallocate - if i > 0 && i < max_realloc_count { + if i == 0 { crate::minicheck::_fail_unless( 0i32, b"/home/sjcrane/projects/c2rust/libexpat/upstream/expat/tests/runtests.c\x00".as_ptr() as *const c_char, 9467i32, - b"Parse requireed reallocation\x00".as_ptr() as *const c_char, + b"Parsing worked despite failing reallocation\x00".as_ptr() as *const c_char, ); } if i >= max_realloc_count { @@ -20722,8 +20715,8 @@ unsafe extern "C" fn context_realloc_test(mut text: *const c_char) { nsalloc_setup(); i += 1 } - // We no longer need to reallocate - if i > 0 && i < max_realloc_count { + // We only reallocate once in xmlparse.rs:getBuffer->try_reserve_exact + if i > 1 && i < max_realloc_count { crate::minicheck::_fail_unless( 0i32, b"/home/sjcrane/projects/c2rust/libexpat/upstream/expat/tests/runtests.c\x00".as_ptr() @@ -20920,8 +20913,7 @@ unsafe extern "C" fn test_nsalloc_realloc_long_ge_name() { nsalloc_setup(); i += 1 } - // We no longer need to reallocate - if i > 0 && i < max_realloc_count { + if i == 0 { crate::minicheck::_fail_unless( 0i32, b"/home/sjcrane/projects/c2rust/libexpat/upstream/expat/tests/runtests.c\x00".as_ptr() From 7dc458477ba2169c1d4e65a98ba4a6e45b6b26fd Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Mon, 3 Aug 2020 01:20:34 -0700 Subject: [PATCH 13/14] Remove unsafe keyword from getBuffer; condense XML_GetBuffer --- src/lib/xmlparse.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/lib/xmlparse.rs b/src/lib/xmlparse.rs index e3db7a59..e819537b 100644 --- a/src/lib/xmlparse.rs +++ b/src/lib/xmlparse.rs @@ -2776,7 +2776,7 @@ pub unsafe extern "C" fn XML_ParseBuffer( impl <'scf> XML_ParserStruct<'scf> { - pub unsafe fn getBuffer(&mut self, len: c_int) -> Option<&mut [c_char]> { + pub fn getBuffer(&mut self, len: c_int) -> Option<&mut [c_char]> { if len < 0 { self.m_errorCode = XML_Error::NO_MEMORY; return None; @@ -2854,16 +2854,13 @@ impl <'scf> XML_ParserStruct<'scf> { } #[no_mangle] -pub unsafe extern "C" fn XML_GetBuffer(mut parser: XML_Parser, mut len: c_int) -> *mut c_void { - if parser.is_null() { - return ptr::null_mut(); - } - - if let Some(buf) = (*parser).getBuffer(len) { - buf.as_mut_ptr() as *mut c_void - } else { - ptr::null_mut() - } +pub extern "C" fn XML_GetBuffer(mut parser: XML_Parser, mut len: c_int) -> *mut c_void { + if let Some(parser) = unsafe{ parser.as_mut() } { + if let Some(buf) = parser.getBuffer(len) { + return buf.as_mut_ptr() as *mut c_void; + } + }; + ptr::null_mut() } /* Stops parsing, causing XML_Parse() or XML_ParseBuffer() to return. Must be called from within a call-back handler, except when aborting From c99e83d513ac58fc40cb4370c14f84daf40e19a1 Mon Sep 17 00:00:00 2001 From: Per Larsen Date: Mon, 3 Aug 2020 03:19:11 -0700 Subject: [PATCH 14/14] stopParser need not be unsafe --- src/lib/xmlparse.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/xmlparse.rs b/src/lib/xmlparse.rs index e819537b..1afe55b9 100644 --- a/src/lib/xmlparse.rs +++ b/src/lib/xmlparse.rs @@ -2895,7 +2895,7 @@ pub extern "C" fn XML_GetBuffer(mut parser: XML_Parser, mut len: c_int) -> *mut */ impl<'scf> XML_ParserStruct<'scf> { - pub unsafe fn stopParser(&mut self, resumable: XML_Bool) -> XML_Status { + pub fn stopParser(&mut self, resumable: XML_Bool) -> XML_Status { match self.m_parsingStatus.parsing { XML_Parsing::SUSPENDED => { if resumable {