On Sat, 2004-02-14 at 22:43, Andreas Bombe wrote: > Currently rb seems to have no signal handler to catch SIGINT or > anything. This means that it dies hard when it gets any termination > signal without shutting down properly. Signals are basically evil, and interact especially poorly with multithreaded applications. One nice thing about Rhythmbox being based purely GTK+ and GNOME is that in theory we can be portable to other platforms like MacOS and Windows, the latter of which doesn't have signals. So I'd rather avoid sprinking #ifdefs about the source and the portability problems that go with that, just to get a nice shutdown if you send SIGINT (which really no normal user should be doing). If you wanted to do this really right, probably add a patch to GTK+ to intercept SIGINT itself, and optionally run a callback in the application. Like g_main_context_set_system_interrupt_handler (...). > I can't see an obvious way to shut down or signal to shut down the > running RBShell in the proper context. Doing anything nontrivial from a signal handler context is a big pain. It might work if you queued an idle function from the handler, but even that is kind of shady if the SIGINT came inside the glib code for queueing a signal handler - it would probably just deadlock, since it would try to re-acquire the already locked context mutex. The safest way to do it would be to set a simple global variable, and have an idle function that polls that every 2 seconds or something. But again, let's just not do it :) It's not worth it. > Something different: The GNOME icon may be set from a NULL pointer. > Granted, only in incomplete installations but it still isn't nice. Ok, merged, thanks: * committed rhythmbox-devel@gnome.org--2004/rhythmbox--main--0.7--patch-85 Ideally we'd be able to work completely uninstalled, but I think that requires some additions to the GNOME library APIs. > Since I didn't get further with the signal stuff this is also the only > patch in aeb@debian.org--2004/rhythmbox--main--0.7 (to be found on > http://home.in.tum.de/~bombe/arch/aeb@debian.org--2004 ) Cool, I added this archive to the development page. > Also, the sources are riddled with whitespace at end of lines, > whitespace only lines and inconsistently mixed spaces/tabs. Sic an > Emacs with develock mode on the sources and have your eyeballs melt. I > suppose fixing that would be very intrusive, resulting in many merge > rejects and the like. Right...at least the mixture of tabs and spaces I suppose is mostly my fault, since I use Emacs (which defaults to that), but I inherited the original codebase from a vim user. There's really two ways to solve this - tabs only, or spaces only. If we're going to change, I'd like to just use spaces only, since it allows for nicer-looking indentation. When people submit patches or make commits, it'd be fine to also change that region of code to spaces only. You're right in that we shouldn't try to do it all at once. I'll add the requisite Emacs magic to make Emacs do spaces-only. Does anyone know the vim magic to do the same?
This is a digitally signed message part