Re: [Rosegarden-devel] [patch] Killing of unning jackd

[Eckhard J. <e.j...@u-...>]

=2D----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Guillaume Laurentwrote :
> On Friday 09 January 2004 14:59, Eckhard Jokisch wrote:
> > As this is my first patch I contribute o an OpenSource project I'm very
> > happy about any feedback knowing that I'll have to learn a lot more ...
>
> I'm sorry but I have to reject the patch, mainly because the for() loop in
> launchJack() initializing the arguments for the jackd process is incorrec=
t.
>
> First of all, you're calling KProcess::start() in it. Calling it once is
> enough. :-)
OK - that's a typo. The "}" went in a wrong line somehow ;-)

>
> Second, you seem to have misunderstood how KProcess handles arguments. Wh=
at
> you did is to split the command line for the config (so far so good), then
> append the 1st element to the kprocess (still ok), but concatenate all
> further args in one string and append this string to the kprocess for each
> arg.
=2E....
Well that's the original code  from CVS ... I dind't lokk at it seriously=20
assuming that it's right
=2E....
>
> :-)
>
> Anyway if you actually tested this patch, I'm quite surprised it could ev=
er
> work. You might have a stray jackd running to which the GUI was connectin=
g.
I appologize for my sloppyness. I just tested that RG wouldn't kill my jack=
=20
process that I started from hydrogen was not killed anymore - in this way i=
t=20
was intended that RG connects to an existing jackd.
>
> Another thing is, if the GUI is going to be able to start jackd then it
> should also connect the KProcess::processExited() signal like it does with
> the sequencer. Also, test for KProcess::isRunning() before attempting to
> connect to jackd.
OK - I will put this in.

>
> Finally, please try to use the same indentation style as the rest of the
> file, or at least the majority of it :
>
> if () {
> }
>
> not
>
> if ()
> {
> }
Will do so
> I'll be happy to include your patch once you fix these issues.
Shall I send it directly to you - as you already looked at it - or to the=20
list?

Thanks for the detailed feedback :-)

Eckhard
=2D----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (GNU/Linux)

iD8DBQFAAAIciF9FD4kCbGQRAno4AJ9Vp3pFIPv2NIn5OC9rykT5zHhQVQCgiVj3
eCapc3jU/1QCYWHofr2AEZM=3D
=3DCQ20
=2D----END PGP SIGNATURE-----