Skip to content

Commit

Permalink
ICU-22934 Fix typo in PR#3230
Browse files Browse the repository at this point in the history 
ICU-22934 Fix error handling while new return nullptr

ICU-22934 Simplfied

ICU-22934 more

ICU-22934 fix leak
  • Loading branch information
@FrankYFTang
FrankYFTang committed Oct 10, 2024
1 parent 5b45e5c commit 5cbbcef
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 35 deletions.
86 changes: 64 additions & 22 deletions icu4c/source/common/rbbinode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ static int gLastSerial = 0;
// Constructor. Just set the fields to reasonable default values.
//
//-------------------------------------------------------------------------
RBBINode::RBBINode(NodeType t) : UMemory() {
RBBINode::RBBINode(NodeType t, UErrorCode& status) : UMemory() {
if (U_FAILURE(status)) {
return;
}
#ifdef RBBI_DEBUG
fSerialNum = ++gLastSerial;
#endif
Expand All @@ -65,10 +68,13 @@ RBBINode::RBBINode(NodeType t) : UMemory() {
fVal = 0;
fPrecedence = precZero;

UErrorCode status = U_ZERO_ERROR;
fFirstPosSet = new UVector(status); // TODO - get a real status from somewhere
fFirstPosSet = new UVector(status);
fLastPosSet = new UVector(status);
fFollowPos = new UVector(status);
if (U_SUCCESS(status) &&
(fFirstPosSet == nullptr || fLastPosSet == nullptr || fFollowPos == nullptr)) {
status = U_MEMORY_ALLOCATION_ERROR;
}
if (t==opCat) {fPrecedence = precOpCat;}
else if (t==opOr) {fPrecedence = precOpOr;}
else if (t==opStart) {fPrecedence = precStart;}
Expand All @@ -77,7 +83,10 @@ RBBINode::RBBINode(NodeType t) : UMemory() {
}


RBBINode::RBBINode(const RBBINode &other) : UMemory(other) {
RBBINode::RBBINode(const RBBINode &other, UErrorCode& status) : UMemory(other) {
if (U_FAILURE(status)) {
return;
}
#ifdef RBBI_DEBUG
fSerialNum = ++gLastSerial;
#endif
Expand All @@ -94,10 +103,13 @@ RBBINode::RBBINode(const RBBINode &other) : UMemory(other) {
fVal = other.fVal;
fRuleRoot = false;
fChainIn = other.fChainIn;
UErrorCode status = U_ZERO_ERROR;
fFirstPosSet = new UVector(status); // TODO - get a real status from somewhere
fLastPosSet = new UVector(status);
fFollowPos = new UVector(status);
if (U_SUCCESS(status) &&
(fFirstPosSet == nullptr || fLastPosSet == nullptr || fFollowPos == nullptr)) {
status = U_MEMORY_ALLOCATION_ERROR;
}
}


Expand Down Expand Up @@ -210,24 +222,37 @@ RBBINode *RBBINode::cloneTree(UErrorCode &status, int depth) {
// If the current node is a variable reference, skip over it
// and clone the definition of the variable instead.
n = fLeftChild->cloneTree(status, depth+1);
if (U_FAILURE(status)) {
return nullptr;
}
} else if (fType == RBBINode::uset) {
n = this;
} else {
n = new RBBINode(*this);
n = new RBBINode(*this, status);
if (U_FAILURE(status)) {
delete n;
return nullptr;
}
// Check for null pointer.
if (n != nullptr) {
if (fLeftChild != nullptr) {
n->fLeftChild = fLeftChild->cloneTree(status, depth+1);
if (U_SUCCESS(status)) {
n->fLeftChild->fParent = n;
}
if (n == nullptr) {
status = U_MEMORY_ALLOCATION_ERROR;
return nullptr;
}
if (fLeftChild != nullptr) {
n->fLeftChild = fLeftChild->cloneTree(status, depth+1);
if (U_FAILURE(status)) {
delete n;
return nullptr;
}
if (fRightChild != nullptr) {
n->fRightChild = fRightChild->cloneTree(status, depth+1);
if (U_SUCCESS(status)) {
n->fRightChild->fParent = n;
}
n->fLeftChild->fParent = n;
}
if (fRightChild != nullptr) {
n->fRightChild = fRightChild->cloneTree(status, depth+1);
if (U_FAILURE(status)) {
delete n;
return nullptr;
}
n->fRightChild->fParent = n;
}
}
return n;
Expand Down Expand Up @@ -265,20 +290,33 @@ RBBINode *RBBINode::flattenVariables(UErrorCode& status, int depth) {
}
if (fType == varRef) {
RBBINode *retNode = fLeftChild->cloneTree(status, depth+1);
if (retNode != nullptr) {
retNode->fRuleRoot = this->fRuleRoot;
retNode->fChainIn = this->fChainIn;
if (U_FAILURE(status)) {
return this;
}
retNode->fRuleRoot = this->fRuleRoot;
retNode->fChainIn = this->fChainIn;
delete this; // TODO: undefined behavior. Fix.
return retNode;
}

if (fLeftChild != nullptr) {
fLeftChild = fLeftChild->flattenVariables(status, depth+1);
if (fLeftChild == nullptr) {
status = U_MEMORY_ALLOCATION_ERROR;
}
if (U_FAILURE(status)) {
return this;
}
fLeftChild->fParent = this;
}
if (fRightChild != nullptr) {
fRightChild = fRightChild->flattenVariables(status, depth+1);
if (fRightChild == nullptr) {
status = U_MEMORY_ALLOCATION_ERROR;
}
if (U_FAILURE(status)) {
return this;
}
fRightChild->fParent = this;
}
return this;
Expand Down Expand Up @@ -312,8 +350,10 @@ void RBBINode::flattenSets(UErrorCode &status, int depth) {
RBBINode *replTree = usetNode->fLeftChild;
fLeftChild = replTree->cloneTree(status, depth+1);
if (U_FAILURE(status)) {
fLeftChild->fParent = this;
delete setRefNode;
return;
}
fLeftChild->fParent = this;
delete setRefNode;
} else {
fLeftChild->flattenSets(status, depth+1);
Expand All @@ -327,8 +367,10 @@ void RBBINode::flattenSets(UErrorCode &status, int depth) {
RBBINode *replTree = usetNode->fLeftChild;
fRightChild = replTree->cloneTree(status, depth+1);
if (U_FAILURE(status)) {
fRightChild->fParent = this;
delete setRefNode;
return;
}
fRightChild->fParent = this;
delete setRefNode;
} else {
fRightChild->flattenSets(status, depth+1);
Expand Down
4 changes: 2 additions & 2 deletions icu4c/source/common/rbbinode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ class RBBINode : public UMemory {
UVector *fFollowPos;


RBBINode(NodeType t);
RBBINode(const RBBINode &other);
RBBINode(NodeType t, UErrorCode& status);
RBBINode(const RBBINode &other, UErrorCode& status);
~RBBINode();
static void NRDeleteNode(RBBINode *node);

Expand Down
15 changes: 12 additions & 3 deletions icu4c/source/common/rbbiscan.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -767,15 +767,24 @@ void RBBIRuleScanner::findSetFor(const UnicodeString &s, RBBINode *node, Unicode
c = s.char32At(0);
setToAdopt = new UnicodeSet(c, c);
}
if (setToAdopt == nullptr) {
error(U_MEMORY_ALLOCATION_ERROR);
return;
}
}

//
// Make a new uset node to refer to this UnicodeSet
// This new uset node becomes the child of the caller's setReference node.
//
RBBINode *usetNode = new RBBINode(RBBINode::uset);
UErrorCode localStatus = U_ZERO_ERROR;
RBBINode *usetNode = new RBBINode(RBBINode::uset, localStatus);
if (usetNode == nullptr) {
error(U_MEMORY_ALLOCATION_ERROR);
localStatus = U_MEMORY_ALLOCATION_ERROR;
}
if (U_FAILURE(localStatus)) {
delete usetNode;
error(localStatus);
delete setToAdopt;
return;
}
Expand Down Expand Up @@ -1191,7 +1200,7 @@ RBBINode *RBBIRuleScanner::pushNewNode(RBBINode::NodeType t) {
return nullptr;
}
fNodeStackPtr++;
fNodeStack[fNodeStackPtr] = new RBBINode(t);
fNodeStack[fNodeStackPtr] = new RBBINode(t, *fRB->fStatus);
if (fNodeStack[fNodeStackPtr] == nullptr) {
*fRB->fStatus = U_MEMORY_ALLOCATION_ERROR;
}
Expand Down
12 changes: 10 additions & 2 deletions icu4c/source/common/rbbisetb.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,11 @@ void RBBISetBuilder::addValToSets(UVector *sets, uint32_t val) {
}

void RBBISetBuilder::addValToSet(RBBINode *usetNode, uint32_t val) {
RBBINode *leafNode = new RBBINode(RBBINode::leafChar);
RBBINode *leafNode = new RBBINode(RBBINode::leafChar, *fStatus);
if (U_FAILURE(*fStatus)) {
delete leafNode;
return;
}
if (leafNode == nullptr) {
*fStatus = U_MEMORY_ALLOCATION_ERROR;
return;
Expand All @@ -388,9 +392,13 @@ void RBBISetBuilder::addValToSet(RBBINode *usetNode, uint32_t val) {
// There are already input symbols present for this set.
// Set up an OR node, with the previous stuff as the left child
// and the new value as the right child.
RBBINode *orNode = new RBBINode(RBBINode::opOr);
RBBINode *orNode = new RBBINode(RBBINode::opOr, *fStatus);
if (orNode == nullptr) {
*fStatus = U_MEMORY_ALLOCATION_ERROR;
}
if (U_FAILURE(*fStatus)) {
delete orNode;
delete leafNode;
return;
}
orNode->fLeftChild = usetNode->fLeftChild;
Expand Down
26 changes: 20 additions & 6 deletions icu4c/source/common/rbbitblb.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,22 @@ void RBBITableBuilder::buildForwardTable() {
// {bof} fake character.
//
if (fRB->fSetBuilder->sawBOF()) {
RBBINode *bofTop = new RBBINode(RBBINode::opCat);
RBBINode *bofLeaf = new RBBINode(RBBINode::leafChar);
// Delete and exit if memory allocation failed.
if (bofTop == nullptr || bofLeaf == nullptr) {
RBBINode *bofTop = new RBBINode(RBBINode::opCat, *fStatus);
if (bofTop == nullptr) {
*fStatus = U_MEMORY_ALLOCATION_ERROR;
}
if (U_FAILURE(*fStatus)) {
delete bofTop;
return;
}
RBBINode *bofLeaf = new RBBINode(RBBINode::leafChar, *fStatus);
// Delete and exit if memory allocation failed.
if (bofLeaf == nullptr) {
*fStatus = U_MEMORY_ALLOCATION_ERROR;
}
if (U_FAILURE(*fStatus)) {
delete bofLeaf;
delete bofTop;
return;
}
bofTop->fLeftChild = bofLeaf;
Expand All @@ -120,18 +129,23 @@ void RBBITableBuilder::buildForwardTable() {
// Appears as a cat-node, left child being the original tree,
// right child being the end marker.
//
RBBINode *cn = new RBBINode(RBBINode::opCat);
RBBINode *cn = new RBBINode(RBBINode::opCat, *fStatus);
// Exit if memory allocation failed.
if (cn == nullptr) {
*fStatus = U_MEMORY_ALLOCATION_ERROR;
}
if (U_FAILURE(*fStatus)) {
delete cn;
return;
}
cn->fLeftChild = fTree;
fTree->fParent = cn;
RBBINode *endMarkerNode = cn->fRightChild = new RBBINode(RBBINode::endMark);
RBBINode *endMarkerNode = cn->fRightChild = new RBBINode(RBBINode::endMark, *fStatus);
// Delete and exit if memory allocation failed.
if (cn->fRightChild == nullptr) {
*fStatus = U_MEMORY_ALLOCATION_ERROR;
}
if (U_FAILURE(*fStatus)) {
delete cn;
return;
}
Expand Down

0 comments on commit 5cbbcef

Please sign in to comment.