You are viewing a plain text version of this content. The canonical link for it is here.
Posted to c-dev@xerces.apache.org by GitBox <gi...@apache.org> on 2021/09/22 20:08:42 UTC
[GitHub] [xerces-c] rouault opened a new pull request #39: Memleak fixes in ContentSpecNode and ComplexTypeInfo classes
rouault opened a new pull request #39:
URL: https://github.com/apache/xerces-c/pull/39
when a OutOfMemory exception occurs.
Spotted by https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=39127 (on GDAL)
The commits are a bit in increasing order of triviality. The ownership rules of ContentSpecNode first and second members, as used by ComplexTypeInfo, are super complex. shared_ptr would be much welcome here! I can just tell that valgrind on my test case reports no double-free nor memory leak after those fixes....
By the way what is the C++11 story of Xerces and the potential use of unique_ptr, shared_ptr ?
--
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-unsubscribe@xerces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: c-dev-help@xerces.apache.org
[GitHub] [xerces-c] rleigh-codelibre edited a comment on pull request #39: [XERCESC-2227] Memleak fixes in ContentSpecNode and ComplexTypeInfo classes
Posted by GitBox <gi...@apache.org>.
rleigh-codelibre edited a comment on pull request #39:
URL: https://github.com/apache/xerces-c/pull/39#issuecomment-926103264
@rouault There were two instances of this, does the other one need fixing as well?
Edit: I'm mistaken I think, please ignore.
--
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-unsubscribe@xerces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: c-dev-help@xerces.apache.org
[GitHub] [xerces-c] rleigh-codelibre commented on pull request #39: [XERCESC-2227] Memleak fixes in ContentSpecNode and ComplexTypeInfo classes
Posted by GitBox <gi...@apache.org>.
rleigh-codelibre commented on pull request #39:
URL: https://github.com/apache/xerces-c/pull/39#issuecomment-925360640
I'll get to the review tomorrow. In terms of C++11 support, the stable release does not require it. The master branch (4.0.x) does. You'll see today I merged a PR from last year which requires C++11 `<cstdint>`. While the CMake build tries to enable C++17, C++14 or C++11 (where available), this was the first feature addition which actually used a C++11 feature. While I don't think there is as of yet much discussion about what could be refactored to use C++11 features, previous discussion has focussed upon retaining API compatibility to the maximum extent possible, so I think we will need to be fairly conservative with changes, certainly changes which affect the API or ABI.
Since Xerces-C++ did not originally use any of the C++ Standard Library, it does have some fairly archaic design which could certainly be replaced. The memory management in particular. However, it does need to work as a Windows DLL which puts constraints upon use of C++ features which cross DLL boundaries--templates in particular are very tricky if not impossible in many cases to export. But the biggest constraint is developer time and interest.
I do have a branch which makes it use `std::thread`. But there are whole categories of wrappers and helpers which could be gutted and replaced with standard equivalents.
--
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-unsubscribe@xerces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: c-dev-help@xerces.apache.org
[GitHub] [xerces-c] rouault commented on pull request #39: [XERCESC-2227] Memleak fixes in ContentSpecNode and ComplexTypeInfo classes
Posted by GitBox <gi...@apache.org>.
rouault commented on pull request #39:
URL: https://github.com/apache/xerces-c/pull/39#issuecomment-926102766
> Where you changed `retNode` to `specNode`, am I correct in the assumption that this was always buggy?
no the code was fine. The change was a result from some temporary confusion, but at line 741 which I believe the only occurence, the 2 were aliases. I've pushed a commit to restore the original use of retNode
--
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-unsubscribe@xerces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: c-dev-help@xerces.apache.org
[GitHub] [xerces-c] rouault commented on pull request #39: [XERCESC-2227] Memleak fixes in ContentSpecNode and ComplexTypeInfo classes
Posted by GitBox <gi...@apache.org>.
rouault commented on pull request #39:
URL: https://github.com/apache/xerces-c/pull/39#issuecomment-948891203
maybe this can be merged ? and https://github.com/apache/xerces-c/pull/40 as well ?
--
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-unsubscribe@xerces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: c-dev-help@xerces.apache.org
[GitHub] [xerces-c] rleigh-codelibre commented on pull request #39: [XERCESC-2227] Memleak fixes in ContentSpecNode and ComplexTypeInfo classes
Posted by GitBox <gi...@apache.org>.
rleigh-codelibre commented on pull request #39:
URL: https://github.com/apache/xerces-c/pull/39#issuecomment-926097632
I have read through all the changes, and everything looks sensible.
Where you changed `retNode` to `specNode`, am I correct in the assumption that this was always buggy?
If anyone else has time to look through, that would be appreciated. I'm certainly in favour of merging it, but given that this in some fairly complex code, it could certainly use a second or third opinion.
--
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-unsubscribe@xerces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: c-dev-help@xerces.apache.org
[GitHub] [xerces-c] rleigh-codelibre merged pull request #39: [XERCESC-2227] Memleak fixes in ContentSpecNode and ComplexTypeInfo classes
Posted by GitBox <gi...@apache.org>.
rleigh-codelibre merged pull request #39:
URL: https://github.com/apache/xerces-c/pull/39
--
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-unsubscribe@xerces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: c-dev-help@xerces.apache.org
[GitHub] [xerces-c] rleigh-codelibre commented on pull request #39: [XERCESC-2227] Memleak fixes in ContentSpecNode and ComplexTypeInfo classes
Posted by GitBox <gi...@apache.org>.
rleigh-codelibre commented on pull request #39:
URL: https://github.com/apache/xerces-c/pull/39#issuecomment-950272435
@rouault Both now merged, thanks!
--
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-unsubscribe@xerces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: c-dev-help@xerces.apache.org
[GitHub] [xerces-c] rleigh-codelibre commented on pull request #39: [XERCESC-2227] Memleak fixes in ContentSpecNode and ComplexTypeInfo classes
Posted by GitBox <gi...@apache.org>.
rleigh-codelibre commented on pull request #39:
URL: https://github.com/apache/xerces-c/pull/39#issuecomment-926103264
@rouault There were two instances of this, does the other one need fixing as well?
--
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-unsubscribe@xerces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: c-dev-help@xerces.apache.org