GNOME Bugzilla – Bug 602072
Allow the debugged program to use the terminal nemiver was launched from
Last modified: 2009-11-25 13:56:00 UTC
Created attachment 147856 [details] [review] Patch to add "Use lauch terminal" option I generally start debugging by doing "<debugger> <program>" from an existing terminal and would like the program being debugged to use that terminal. The attached patch adds an option to nemiver to make the debugged program use the terminal where nemiver was launched instead of the terminal builtin to nemiver. If nemiver is not launched from a terminal then it falls back to using the builtin terminal. I've called the option "use launch terminal" at the moment but I freely admit that it doesn't seem a particularly good name so I'm open to suggestions for an alternative...
Created attachment 148253 [details] [review] Updated patch Hello Tom, Thank you for your patch, and sorry for the delay in reviewing it. The patch looks good, thank you for that. I have a couple of little points I'd like to discuss, however. * In src/dbgengine/nmv-gdb-engine.cc I have factored the other instances of the "set inferior tty" command that were used, to make them use GDBEngine::Priv::set_tty_path. I made your GDBEngine::set_tty_path use it as well. * I have added a --use-launch-terminal command line switch that would let the inferior use the launch terminal, without having to set the persistent conf key. * I have switched the "launch terminal" conf key off by default, because I have the impression it's what most people are used to. This is arguable, of course. * I noticed GDB spits error: "GDB: Failed to set controlling terminal: operation not permitted" upon startup, using this new option. I dug a bit and commented by findings in main.cc, in the updated patch. In short, I think we have to make Nemiver release its controlling terminal, if we want the inferior to use it too. This might be a bug in GDB though. I need to sort that out later. For now, I am just making Nemiver release its controlling terminal, which a little bit annoying because we can't quit nemiver by hitting ctrl-c in its launch terminal anymore, when using the --use-launch-terminal option. Thoughts ? * I have added a GNU/GNOME style of ChangeLog to comply with what we've used to do historically. The attached patch contains this little additions. Thoughts?
All sounds good to me. I had always intended it to default to off - it was accidental that it didn't. I'm still getting those messages from gdb about the controlling terminal, so it sounds like there is something else that needs doing there...
> All sounds good to me. I had always intended it to default to off - it was > accidental that it didn't. No problem. Thanks for the feedback. > I'm still getting those messages from gdb about the controlling terminal, so it > sounds like there is something else that needs doing there... Ah, are you getting the error message when using the --use-launch-terminal switch ? I am not getting it using the switch. But if you don't use the cmd line switch and are using just the gconf key instead, then you'll get the error message because I am not calling setsid() on that code path yet. I wanted to get the setsid() call tested a little bit before wiring it completely.
(In reply to comment #3) > Ah, are you getting the error message when using the --use-launch-terminal > switch ? > I am not getting it using the switch. > > But if you don't use the cmd line switch and are using just the gconf key > instead, then you'll get the error message because I am not calling setsid() on > that code path yet. I wanted to get the setsid() call tested a little bit > before wiring it completely. I'm getting it in both cases - the reason is that the setsid() call is failing with EPERM. The reason it's failing is that you can't create a session with setsid() if you're already a process group leader and nemiver is already a process group leader - that may well be down to my shell (I'm using zsh) choosing to put everything it starts in a new process group. I thought using ioctl(0, TIOCNOTTY, 0) instead might help, but that doesn't seem to work either...
(In reply to comment #4) > I'm getting it in both cases - the reason is that the setsid() call is failing > with EPERM. > > The reason it's failing is that you can't create a session with setsid() if > you're already a process group leader and nemiver is already a process group > leader - that may well be down to my shell (I'm using zsh) choosing to put > everything it starts in a new process group. Ohhh, correct. I overlooked that. I am using bash from wihting GNU Screen. In that case, the shell doesn't put each job in its own process group. Using bash from, say, gnome-terminal does seem to set Nemiver as the process leader too. > I thought using ioctl(0, TIOCNOTTY, 0) instead might help, but that doesn't > seem to work either... Sigh. Since the comment is harmles, I'll just commit the patch as is for now, leaving the comment in main.c I'll try to investigate this later. Thank you.
Commited this to master the other day, but forgot to update the status here. http://git.gnome.org/cgit/nemiver/commit/?id=7643c579f35efbb3012c9b23002b3a649f5f0455 It should appear in 0.7.3. Thanks!