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

List:       kde-multimedia
Subject:    Re: PATCH: fix for #29008
From:       Stefan Westerfeld <stefan () space ! twc ! de>
Date:       2001-07-23 19:50:53
[Download RAW message or body]

   Hi!

On Mon, Jul 23, 2001 at 11:13:44AM +0200, Nikolas Zimmermann wrote:
> On Monday 23 July 2001 00:17, Stefan Westerfeld wrote:
> > Due to KArtsDispatcher::free() being called before the m_soundServer
> > destructor in konq_sound.cc, there is a crash. I think the most reasonable
> > way to fix this is to change KArtsDispatcher in a way that ::init and
> >
> > ::free are replaced by the constructor and destructor.
> hm hmm hmm don't like that somehow
> >
> > That way, you will have to create an instance of KArtsDispatcher, but I
> > think this is a good thing anyway, because:
> i thought we wanted to avoid that
> >
> >  - it makes #29008 easy to fix
> >  - it ensures that every init is paired with a free if you
> >     - create KArtsDispatcher on the stack
> > 	- create KArtsDispatcher as member of a class
> > 	- use Qt object autodeletion
> >  - it works more like the "classic" dispatcher
> i didn't want that :) that's why i've made it a Singleton.

You could have used a namespace (it's not even a class, actually, just two
global functions). The constructor and destructor in your code do nothing,
so you never need an instance. You also need not inherit QObject therefore.

> >  - we can later put signals into KArtsDispatcher if we need to
> if, then else...... do we really need that?
> Why?
> 
> >
> Why not just fix konq_sound??

Well, I tried doing so first, because it sounds like the natural thing to
do, but I found no elegant way of doing so. Basically, if you have

// the following code is broken:
class Foo {
public:
	Arts::SoundServer m_server;
	Foo() { KArtsDispatcher::init(); m_server = something; }
	~Foo() { KArtsDispatcher::free(); }
};

then what you really want is to

 - call KArtsDispatcher::init() *before* m_server gets initialized
 - call KArtsDispatcher::free() *after* m_server gets destroyed

but as Foo() gets executed after m_server gets initialized and ~Foo() gets
executed before m_server gets destroyed, it's not obvious how to fix it with
the API KArtsDispatcher currently has.

So what do you suggest as fix?

class Foo {
public:
	Arts::SoundServer *m_server;
	Foo() { KArtsDispatcher::init(); m_server = new Arts::SoundServer; *m_server = something; }
	~Foo() { delete m_server; KArtsDispatcher::free(); }
} ?

I really hate this, because smartwrappers were not designed to use them with
new and delete - it's quite contraintuitive to do this, because a smartwrapper
is actually something like a pointer, so a pointer to a smartwrapper is more
like a pointer to a pointer...

class Foo {
public:
	struct Internals {
		Arts::SoundServer m_server;
	} *i;
	Foo() { KArtsDispatcher::init(); i = new Internals(); i->m_server = something; }
	~Foo() { delete i; KArtsDispatcher::free(); }
};

Well... not my case either. It looks better because at least you can use
m_server in the ordinary way, but the Internals (or if you like FooPrivate)
thing still looks artificial. As a new programmer, you might not do the right
thing at the first try, and you might think "oh well, it's one of these aRts
bugs again", once you hit the assertion.

So IMHO

class Foo {
public:
	KArtsDispatcher m_dispatcher;
	Arts::SoundServer m_server;

	Foo() { m_server = something; }
	~Foo() { }
};

looks best, but requires changing KArtsDispatcher in the way I suggested.

So what do you suggest?

   Cu... Stefan
-- 
  -* Stefan Westerfeld, stefan@space.twc.de (PGP!), Hamburg/Germany
     KDE Developer, project infos at http://space.twc.de/~stefan/kde *-         
_______________________________________________
Kde-multimedia mailing list
Kde-multimedia@master.kde.org
http://master.kde.org/mailman/listinfo/kde-multimedia

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