Hi Ola, > > A DTDEntityDecl object is allocated and pushed into the ReaderMgr stack. > > ReaderMgr does not own the stack's content, so objects neither get freed on > > ReaderMgr::popReader(), nor on ReaderMgr::~ReaderMgr(). > > And it should not be freed by the code popping the object?
I don't think so. In fact, the code popping the object is the ReaderMgr
itself: popReader is a private method called by a higher level scanning
API, e.g. ReaderMgr::getNextChar.
$ ag popReader
src/xercesc/internal/ReaderMgr.cpp
107: if (!popReader())
129: if (!popReader())
149: if (!popReader())
166: if (!popReader())
191: if (!popReader())
214: if (!popReader())
237: if (!popReader())
256: if (!popReader())
273: if (!popReader())
1056:bool ReaderMgr::popReader()
src/xercesc/internal/ReaderMgr.hpp
203: bool popReader();
> > A janitor is set to avoid leaking memory via the allocated DTDEntityDecl.
> > Unfortunately the object is freed when the janitor gets out of scope, at
> > which point a pointer to this object is still present within the stack.
>
> Do you mean that the janitor should not free it at this point?
Exactly. This is too early!
> <snip>
> # I suggest the following fix:
> >
> > Add a `bool adopt` parameter to ReaderMgr::pushReader() (default value of
> > false), and a `RefStackOf<XMLEntityDecl>* fAdoptedStack` private element to
> > the ReaderMgr class.
> >
> > Whenever ReaderMgr::pushReader() is called with `adopt` set to true, also
> > push passed object onto `fAdoptedStack`.
> >
> > Whenever ReaderMgr::popReader() is called, check whether the object being
> > removed is on top of `fAdoptedStack`. If so, remove and delete it.
> >
> > On ReaderMgr::~ReaderMgr(), delete `fAdoptedStack` and all possibly
> > remaining elements.
> >
>
> I assume that you also remove the janitor in this case?
Yes!
Thanks for having a look. Did I manage to answer your questions? If so, I'd
go on, implement the patch and try to get some review from upstream.
BTW, I don't think previous messages reached the c-dev mailing list. I
tried to subscribe, let's see if this one does.
FTR: initial message here[0].
cheers,
Hugo
[0] https://lists.debian.org/debian-lts/2020/01/msg00055.html
--
Hugo Lefeuvre (hle) | www.owl.eu.com
RSA4096_ 360B 03B3 BF27 4F4D 7A3F D5E8 14AA 1EB8 A247 3DFD
ed25519_ 37B2 6D38 0B25 B8A2 6B9F 3A65 A36F 5357 5F2D DC4C
signature.asc
Description: PGP signature
