[prev in list] [next in list] [prev in thread] [next in thread] 

List:       mico-devel
Subject:    Re: [mico-devel] trader ported to boa
From:       Karel Gardas <kgardas () objectsecurity ! com>
Date:       2004-04-21 21:01:40
Message-ID: Pine.LNX.4.43.0404212239121.514-100000 () thinkpad ! wg-ro-gar1 ! inext ! cz
[Download RAW message or body]

On Wed, 21 Apr 2004, Joerg Knobloch wrote:

> | 1) when you see somewhere --no-poa, --no-boa, --poa or --boa as an IDL
> | compiler command-line option used, please remove this, since --poa
> | --no-boa is/was default for a long time now, so --poa is useless and
> | --no-boa also since there is no BOA at all in mico--boa-removal branch
> | where your change will be merged.
> Ok, removed them for traderd, event, trader and dynany demos. For dynany
> demo I also removed boa inside.

OK. Here I would like to add one note. Please do not mix patches of
different kind. i.e. you do something, but then you also fix some warnings
(my guess, since changes int -> unsigned int, CORBA::Long -> unsigned long
seems so), but you commit it in _one_ patch. Those changes should be two
different patches, since then it's more easy to cherry-pick just imporant
patches from your archive and leave other... Anyway, please note that I'm
_very_ glad that you use Arch! I just do tla library-add
joerg@happypenguins.net--devel/mico--devel--2.3 and then use xxdiff
(http://xxdiff.sf.net) and see what you have changed between base-0 and
patch-X and I can very quickly overview your all changes... ;-)

> | 2) you use code like return <class>::_duplicate(<class instance
> | variable>->_this()), which is IMHO wrong, since this will lead to memory
> | leak. Please note that:
> Whoops. Mea culpa...just removed the duplications. Thanks for pointing
> that out.

You have forgotten to fix this issue in: demo/services/events/server.cc
and demo/services/events/server2.cc

Now I would like to add some other notes:

1) CORBA-SMALL.h -> CORBA.h transition: if you do this in some files, I
guess you would like to resolve compilation failure for unknown POA
related symbols, right? So if you would like to solve this, please
do either: change for inclusion of CORBA.h and remove all #define
MICO_CONF_* before this include statement or (which is better) just add
#define MICO_CONF_POA before include statement and this should solve this
issue. I'm talking here about demo/dynany/dynany.cc,
demo/services/events/server.cc, demo/services/events/server2.cc,
demo/services/events/server3.cc files.

2) coss/traderd/trader_main.cc: void OfferIterator::destroy doesn't like
right, i.e. CORBA::release(this->_this()); will not have any effect IMHO.

3) you have also changed =tagging-method file during commit of some patch.
Please do not do it in normal patch (vide recommendation for not mixing
patches above). Also please note that I will do the same on my mico-devel
branch and so you will be able to get the change from my branch soon.
Maybe we should discuss this file on this mailing list for better
suitability for more MICO's Arch-based developers.

Thanks for your work!

Karel
--
Karel Gardas                  kgardas@objectsecurity.com
ObjectSecurity Ltd.           http://www.objectsecurity.com

_______________________________________________
Mico-devel mailing list
Mico-devel@mico.org
http://www.mico.org/mailman/listinfo/mico-devel
[prev in list] [next in list] [prev in thread] [next in thread]