theta682 commented on code in PR #54:
URL: https://github.com/apache/xerces-c/pull/54#discussion_r1416808653
##########
src/xercesc/internal/ReaderMgr.cpp:
##########
@@ -1020,7 +1070,9 @@ ReaderMgr::getLastExtEntity(const XMLEntityDecl*&
itsEntity) const
// search the stack; else, keep the reader that we've got since its
// either an external entity reader or the main file reader.
//
- const XMLEntityDecl* curEntity = fCurEntity;
+ const XMLEntityDecl* curEntity =
+ fCurReaderData? fCurReaderData->getEntity() : 0;
Review Comment:
```suggestion
fCurReaderData? fCurReaderData->getEntity() : 0;
```
##########
src/xercesc/internal/ReaderMgr.hpp:
##########
@@ -208,36 +214,96 @@ private :
ReaderMgr(const ReaderMgr&);
ReaderMgr& operator=(const ReaderMgr&);
+ // -----------------------------------------------------------------------
+ // Private data types
+ // -----------------------------------------------------------------------
+ class ReaderData : public XMemory
+ {
+ public :
+ // ---------------------------------------------------------------------
+ // Constructors and Destructor
+ // ---------------------------------------------------------------------
+ ReaderData
+ ( XMLReader* const reader
+ , XMLEntityDecl* const entity
+ , const bool adoptEntity
+ );
+
+ ~ReaderData();
+
+ // ----------------------------------------------------------------------
+ // Getter methods
+ // ----------------------------------------------------------------------
+ XMLReader* getReader() const;
+ XMLEntityDecl* getEntity() const;
+ bool getEntityAdopted() const;
+
+ XMLEntityDecl* releaseEntity();
Review Comment:
```suggestion
//
---------------------------------------------------------------------
// Constructors and Destructor
//
---------------------------------------------------------------------
ReaderData
( XMLReader* const reader
, XMLEntityDecl* const entity
, const bool adoptEntity
);
~ReaderData();
//
----------------------------------------------------------------------
// Getter methods
//
----------------------------------------------------------------------
XMLReader* getReader() const;
XMLEntityDecl* getEntity() const;
bool getEntityAdopted() const;
XMLEntityDecl* releaseEntity();
```
##########
src/xercesc/internal/ReaderMgr.hpp:
##########
@@ -208,36 +214,96 @@ private :
ReaderMgr(const ReaderMgr&);
ReaderMgr& operator=(const ReaderMgr&);
+ // -----------------------------------------------------------------------
+ // Private data types
+ // -----------------------------------------------------------------------
+ class ReaderData : public XMemory
+ {
+ public :
+ // ---------------------------------------------------------------------
+ // Constructors and Destructor
+ // ---------------------------------------------------------------------
+ ReaderData
+ ( XMLReader* const reader
+ , XMLEntityDecl* const entity
+ , const bool adoptEntity
+ );
+
+ ~ReaderData();
+
+ // ----------------------------------------------------------------------
+ // Getter methods
+ // ----------------------------------------------------------------------
+ XMLReader* getReader() const;
+ XMLEntityDecl* getEntity() const;
+ bool getEntityAdopted() const;
+
+ XMLEntityDecl* releaseEntity();
+
+ private :
+ // ---------------------------------------------------------------------
+ // Unimplemented constructors and operators
+ // ---------------------------------------------------------------------
+ ReaderData();
+ ReaderData(const ReaderData&);
+ ReaderData& operator=(const ReaderData&);
+
+ // ---------------------------------------------------------------------
+ // Private data members
+ //
+ // fReader
+ // This is the pointer to the reader object that must be destroyed
+ // when this object is destroyed.
+ //
+ // fEntity
+ // fEntityAdopted
+ // This is the pointer to the entity object that, if adopted, must
+ // be destroyed when this object is destroyed.
+ //
+ // Note that we need to keep up with which of the pushed readers
+ // are pushed entity values that are being spooled. This is done
+ // to avoid the problem of recursive definitions.
+ // ---------------------------------------------------------------------
+ XMLReader* fReader;
+ XMLEntityDecl* fEntity;
+ bool fEntityAdopted;
Review Comment:
```suggestion
//
---------------------------------------------------------------------
// Unimplemented constructors and operators
//
---------------------------------------------------------------------
ReaderData();
ReaderData(const ReaderData&);
ReaderData& operator=(const ReaderData&);
//
---------------------------------------------------------------------
// Private data members
//
// Reader
// This is the pointer to the reader object that must be
destroyed
// when this object is destroyed.
//
// fEntity
// fEntityAdopted
// This is the pointer to the entity object that, if adopted,
must
// be destroyed when this object is destroyed.
//
// Note that we need to keep up with which of the pushed readers
// are pushed entity values that are being spooled. This is done
// to avoid the problem of recursive definitions.
//
---------------------------------------------------------------------
XMLReader* Reader;
XMLEntityDecl* fEntity;
bool fEntityAdopted;
```
##########
src/xercesc/internal/ReaderMgr.cpp:
##########
@@ -873,33 +921,50 @@ bool ReaderMgr::isScanningPERefOutOfLiteral() const
return false;
}
-
bool ReaderMgr::pushReader( XMLReader* const reader
, XMLEntityDecl* const entity)
+{
+ return pushReaderAdoptEntity(reader, entity, false);
+}
+
+bool ReaderMgr::pushReaderAdoptEntity( XMLReader* const reader
+ , XMLEntityDecl* const entity
+ , const bool adoptEntity)
{
//
// First, if an entity was passed, we have to confirm that this entity
- // is not already on the entity stack. If so, then this is a recursive
+ // is not already on the reader stack. If so, then this is a recursive
// entity expansion, so we issue an error and refuse to put the reader
// on the stack.
//
// If there is no entity passed, then its not an entity being pushed, so
// nothing to do. If there is no entity stack yet, then of coures it
// cannot already be there.
//
- if (entity && fEntityStack)
+ if (entity && fReaderStack)
{
- const XMLSize_t count = fEntityStack->size();
+ // @@ Strangely, we don't check the entity at the top of the stack
+ // (fCurReaderData). Is it a bug?
+ //
+ const XMLSize_t count = fReaderStack->size();
const XMLCh* const theName = entity->getName();
for (XMLSize_t index = 0; index < count; index++)
{
- const XMLEntityDecl* curDecl = fEntityStack->elementAt(index);
+ const XMLEntityDecl* curDecl =
+ fReaderStack->elementAt(index)->getEntity();
+
if (curDecl)
{
if (XMLString::equals(theName, curDecl->getName()))
{
- // Oops, already there so delete reader and return
+ // Oops, already there so delete reader and entity and
+ // return.
+ //
delete reader;
+
+ if (adoptEntity)
+ delete entity;
Review Comment:
```suggestion
delete entity;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: c-dev-unsubscr...@xerces.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscr...@xerces.apache.org
For additional commands, e-mail: c-dev-h...@xerces.apache.org
