Re: [Libreoffice] [PATCH 05/11] Replace SmNodeStack with std::stack< SmNode* >

On 12/01/2011 11:09 AM, Marcel Metz wrote:
> ---
>   starmath/inc/parse.hxx           |    3 +-
>   starmath/source/math_pch.cxx     |    1 -
>   starmath/source/mathmlimport.cxx |  223
> ++++++++++++++++++++++----------------
>   starmath/source/mathmlimport.hxx |    8 ++-
>   starmath/source/parse.cxx        |  222
> ++++++++++++++++++++++++--------------
>   5 files changed, 279 insertions(+), 178 deletions(-)

sm_unoapi subsequenttest keeps crashing for me now at

> #14 0x0000003e04236285 in __GI_raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> #15 0x0000003e04237b9b in __GI_abort () at abort.c:91
> #16 0x00007fa051d3537d in __gnu_debug::_Error_formatter::_M_error() const () from 
> /data/lo/core/solver/unxlngx6/installation/opt/program/../ure-link/lib/libstdc++.so.6
> #17 0x00007fa02e12aa6f in std::stack<SmNode*, std::__debug::deque<SmNode*, std::allocator<SmNode*> > 
> >::top (this=0x7fa02407a6b8) at 
> /usr/lib/gcc/x86_64-redhat-linux/4.6.2/../../../../include/c++/4.6.2/bits/stl_stack.h:160
> #18 0x00007fa02e157f1d in SmParser::Expression (this=0x7fa02407a690) at 
> /data/lo/core/starmath/source/parse.cxx:1118
> #19 0x00007fa02e157acb in SmParser::Align (this=0x7fa02407a690) at 
> /data/lo/core/starmath/source/parse.cxx:1062
> #20 0x00007fa02e157c75 in SmParser::Line (this=0x7fa02407a690) at 
> /data/lo/core/starmath/source/parse.cxx:1087
> #21 0x00007fa02e15776a in SmParser::Table (this=0x7fa02407a690) at 
> /data/lo/core/starmath/source/parse.cxx:992
> #22 0x00007fa02e15c52a in SmParser::Parse (this=0x7fa02407a690, rBuffer=...) at 
> /data/lo/core/starmath/source/parse.cxx:2483
> #23 0x00007fa02e108677 in SmDocShell::Parse (this=0x7fa02407a510) at 
> /data/lo/core/starmath/source/document.cxx:259

i.e., at SmNode *pNode = m_aNodeStack.top() in SmParser::Expression(), 
where m_aNodeStack is empty.  It *looks* to me that the following 
happens:  The stack is rightfully empty, but the old implementation of 
tools' stack Pop() always returned null in that case, and the code in 
SmParser::Expression() relies on that -- which would explain the 
following if (pNode) clause.  Old tools' stack Pop() was implemented as 
Container::Remove(Count()-1), where the parameter to remove is of 
unsigned sal_uIntPtr, and Container::Remove doing if ( nCount <= nIndex 
) return NULL;

<http://cgit.freedesktop.org/libreoffice/core/commit/?id=b419bb21a76c48dda9f9a6ef6182b98e03358176> 
lets the test succeed, but who knows which other code depended on this 
magnificent misfeature of the old stack implementation, too...

Stephan