Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2012-01-03 Thread Markos Zaharioudakis
Review: Approve


-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2012-01-03 Thread Markos Zaharioudakis
The ExtFuncArgItemSequence cannot live longer than the ExtFunctionCallIterator 
who created it, because it is just a wrapper over a child of the 
ExtFunctionCallIterator, and if the ExtFunctionCallIterator is destroyed, its 
children are destroyed as well.

The real problem is that the ExtFuncArgItemSequence is assigned to an 
ItemSequence_t (the return value of the evaluate() method). Since 
ItemSequence_t is a smart pointer, it will destroy the ExtFuncArgItemSequence 
when itself is destroyed. Later, when the ExtFunctionCallIterator is closed, it 
will also try to destroy the ExtFuncArgItemSequence.

Your fix solves this problem, so I have approved it. 


 Did you also try the example (without my fix) and valgrind? I don't get a
 crash either but valgrind shows that there are serious problems:
 
 ==12383== Invalid read of size 4
 ==12383==at 0x448BDFF: zorba::SmartPtrzorba::ItemSequence::~SmartPtr()
 (smart_ptr.h:82)
 ==12383==by 0x4D25F43:
 zorba::ExtFunctionCallIteratorState::~ExtFunctionCallIteratorState()
 (fncall_iterator.cpp:625)
 ==12383==by 0x4D2FE87: zorba::StateTraitsImplzorba::ExtFunctionCallIterat
 orState::destroyState(zorba::PlanState, unsigned int) (plan_iterator.h:283)
 ==12383==by 0x4D2FE4D:
 zorba::NaryBaseIteratorzorba::ExtFunctionCallIterator,
 zorba::ExtFunctionCallIteratorState::closeImpl(zorba::PlanState)
 (narybase.h:164)
 ==12383==by 0x4D2F815:
 zorba::Batcherzorba::ExtFunctionCallIterator::close(zorba::PlanState)
 (plan_iterator.h:563)
 ==12383==by 0x4CAFC27: zorba::PlanWrapper::close() (plan_wrapper.cpp:179)
 ==12383==by 0x4457EFB: zorba::XQueryImpl::execute(std::ostream,
 Zorba_SerializerOptions const*) (xqueryimpl.cpp:1124)
 ==12383==by 0x4459B13: zorba::operator(std::ostream,
 zorba::SmartPtrzorba::XQuery const) (xqueryimpl.cpp:1580)
 ==12383==by 0x8068B83: external_function_test_4(zorba::Zorba*)
 (external_function.cpp:361)
 ==12383==by 0x8068E6B: external_function(int, char**)
 (external_function.cpp:407)
 ==12383==by 0x80613C4: main (UnitTests.cpp:270)
 ==12383==  Address 0x7e00e28 is 0 bytes inside a block of size 24 free'd
 ==12383==at 0x4025907: operator delete(void*) (vg_replace_malloc.c:387)
 ==12383==by 0x4D2F3F5:
 zorba::ExtFuncArgItemSequence::~ExtFuncArgItemSequence()
 (fncall_iterator.cpp:539)
 ==12383==by 0x4D25F16:
 zorba::ExtFunctionCallIteratorState::~ExtFunctionCallIteratorState()
 (fncall_iterator.cpp:633)
 ==12383==by 0x4D2FE87: zorba::StateTraitsImplzorba::ExtFunctionCallIterat
 orState::destroyState(zorba::PlanState, unsigned int) (plan_iterator.h:283)
 ==12383==by 0x4D2FE4D:
 zorba::NaryBaseIteratorzorba::ExtFunctionCallIterator,
 zorba::ExtFunctionCallIteratorState::closeImpl(zorba::PlanState)
 (narybase.h:164)
 ==12383==by 0x4D2F815:
 zorba::Batcherzorba::ExtFunctionCallIterator::close(zorba::PlanState)
 (plan_iterator.h:563)
 ==12383==by 0x4CAFC27: zorba::PlanWrapper::close() (plan_wrapper.cpp:179)
 ==12383==by 0x4457EFB: zorba::XQueryImpl::execute(std::ostream,
 Zorba_SerializerOptions const*) (xqueryimpl.cpp:1124)
 ==12383==by 0x4459B13: zorba::operator(std::ostream,
 zorba::SmartPtrzorba::XQuery const) (xqueryimpl.cpp:1580)
 ==12383==by 0x8068B83: external_function_test_4(zorba::Zorba*)
 (external_function.cpp:361)
 ==12383==by 0x8068E6B: external_function(int, char**)
 (external_function.cpp:407)
 ==12383==by 0x80613C4: main (UnitTests.cpp:270)
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2011-12-22 Thread Matthias Brantner
Review: Approve


-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2011-12-22 Thread Zorba Build Bot
Voting does not meet specified criteria. Required: Approve  1, Disapprove  1. 
Got: 1 Approve, 2 Pending.
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/bug907872 into lp:zorba

2011-12-22 Thread Markos Zaharioudakis
Matthias, I cannot see how the example you added illustrates the case where an 
ExtFuncArgItemSequence must live longer than the ExtFunctionCallIterator who 
created it. In fact, I tried the example without your modification in 
fncall_iterator.cpp and it worked fine. What am I missing?
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug907872/+merge/86738
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp