Remove the functions UVector::addElementX() and UVector::ensureCapacityX() that
were temporarily added to aid in the cleaning up of UVector's out-of-memory
error handling.
Clean up some oversights from commit 0ec329c. This was triggered by fuzz testing finding
a memory leak with the original commit, see https://oss-fuzz.com/testcase-detail/4656781834452992
I was unable to replicate the fuzzing failure, but reviewing the nearby code showed
some likely problems.
In this commit,
- Fix UStack::pop() to not delete the popped element when a deleter function is present.
This was a bug, but because there were no stacks with deleters, was not causing trouble.
- Change RegexCompile::compileSet() to delete the set if it cannot be added to the internal
vector of sets. I suspect this is the cause of the fuzzing leak - 0ec329c changed the
behavior of UVector in the presence of errors.
- Changed RegexCompile::fSetStack to use an object deleter function. This fixes the
leak checking at the points new elements are pushed onto this stack.
This is the next installment of UVector error handling cleanup. It includes:
- Revise UStack to follow the conventions of UVector, to leave the stack
unmodified when there is an incoming error code. And, for stacks with a
deleter function, to delete the incoming object if it cannot be
succesfully pushed.
- Review all useage of UStack in ICU; adjust call sites as needed.
- Review all uses of UVector::addElementX() in the implementation of
- Regular Expressions
- Break Iteration
- toolutil/xmlparser
changing to the updated functions, and adjusting call sites as needed.
- Add updated versions of UVector::addElement() and ensureCapacity() that respect
incoming errors.
Follow on to c26aebe, which renamed the original versions.
- Add UVector::adoptElement() as a replacement for addElement() when the vector
has a deleter function set, meaning that it adopts ownership of its elements.
The intent is to make the behavior clearer at the call sites when looking
at unfamiliar code.
- Make all functions with an incoming failure, as indicated by a UErrorCode parameter,
leave the vector unchanged.
- Change all functions that store object pointers into the vector such that,
when the store cannot be completed for any reason _and_ the vector has a deleter function,
then the incoming object is deleted.
This change can simplify the error handling code around calls to the affected functions
(addElement() and insertElementAt(), in particular)
- Add index bounds checking on functions where it was possible - that is, on functions
with both U_ErrorCode and index parameters.
- Changed to more modern C++ idioms in some parts of the UVector implementation.
- Review & update as required all uses of the UVector functions
setElementAt(), insertElementAt(), setSize(), sortedInsert()
these being the functions with changed behavior on error conditions
(aside from addElement()).
This PR will be followed by more, switching call sites in various ICU services
from UVector::addElementX() (old behavior on errors)
to UVector::addElement() (new behavior on errors)
This is the first step towards improving the error handling and out-of-memory
behavior of UVector::addElement(). A followup PR will add back a new addElement()
with corrected error handling, then additional followups will switch call sites
from the original (renamed) function to the new addElement().
This commit includes no logic or behavior changes; it only renames the existing functions.
Methods implementead as 'inline' but not declared 'inline' cause clang++
to throw compilation warnings on Windows. This adds 'inline' to the
relevant method declarations.
Use the int32_t functions when you are only using integers.
Also arrays of scalars need to be allocated with malloc (Needed by Lotus j1733).
X-SVN-Rev: 8289
