Hi Martijn, Martijn van Duren wrote on Thu, Aug 12, 2021 at 04:37:24PM +0200:
> Maybe I've used cdio once or twice and I don't have a cd-player at hand > (at least connected to my workstation) to test this. So purely from code > inspection: You set the signal handler before entering el_gets and you > ignore it right after. More precisely, i restore default signal handling right after, which is "terminate process" for SIGINT. In most situations, i dislike ignoring SIGINT. > Wouldn't this imply that if you do something like "cdplay" at the prompt > and while you wait for the cd to spin you hit ^C the application just > exits? Yes. > If so, wouldn't it make more sense to install the signal handler once > and let open_cd() handle EINTR as well and just return to the prompt? Maybe. The question whether cdio(1) should allow Ctrl-C to be used to interrupt more operations, and not just command line editing, and return to the interactive prompt in that case is certainly legitimate. Then again, my point is to unify line editing across different programs, not to consider overall signal handling in cdio(1). As long as we only touch line editing in cdio(1), that goal can be reached and the risk of regressions is easier to control. Extending signal handling over larger swaths of code requires reviewing more code and making sure what Ctrl-C does is sane in all code paths in all of that code. For example, looking at the function run(), i see that most commands start by calling open_cd(), but not all do. A few do not call it at all (e.g. CMD_SET, CMD_HELP), in which case we would have to make sure that the signal handler does not remain installed longer than intended, a few do some other work before calling it (e.g. CMD_DEVICE). So i would prefer to leave the broader question of signal handling in cdio(1), outside the specific domain of command line editing, to people who are more familiar with the code and usage of cdio(1). If somebody wants to develop, test and propose a patch with a wider scope, for example a global signal handler that can interupt any operation - such a patch would have to make sure cleanup is done as required after all operations that might get aborted -, i'm not opposed to that, but it don't feel like developing such a patch myself at this point. Yours, Ingo
