2004-11-15 03:31:48

by Chris Wedgwood

[permalink] [raw]
Subject: [PATCH] uml: fail xterm_open when we have no $DISPLAY

If UML wants to open an xterm channel and the xterm does not run
properly (eg. terminates soon after starting) we will get a hang.
This avoids the most common cause for this and adds a comment (which
long term will go away with a rewrite of that code I guess?)

Signed-off-by: Chris Wedgwood <[email protected]>
---

arch/um/drivers/xterm.c | 7 +++++++
arch/um/drivers/xterm_kern.c | 3 +++
2 files changed, 10 insertions(+)

Index: cw-current/arch/um/drivers/xterm.c
===================================================================
--- cw-current.orig/arch/um/drivers/xterm.c 2004-11-14 17:52:05.955194653 -0800
+++ cw-current/arch/um/drivers/xterm.c 2004-11-14 17:52:23.048945524 -0800
@@ -94,6 +94,13 @@
"/usr/lib/uml/port-helper", "-uml-socket",
file, NULL };

+ /* Check that DISPLAY is set, this doesn't guarantee the xterm
+ * will work but w/o it we can be pretty sure it won't. */
+ if (!getenv("DISPLAY")) {
+ printk("xterm_open: $DISPLAY not set.\n");
+ return -ENODEV;
+ }
+
if(os_access(argv[4], OS_ACC_X_OK) < 0)
argv[4] = "port-helper";

Index: cw-current/arch/um/drivers/xterm_kern.c
===================================================================
--- cw-current.orig/arch/um/drivers/xterm_kern.c 2004-11-14 17:52:51.484194558 -0800
+++ cw-current/arch/um/drivers/xterm_kern.c 2004-11-14 17:53:09.197972625 -0800
@@ -61,6 +61,9 @@
ret = err;
goto out;
}
+
+ /* XXX Note, if the xterm doesn't work for some reason
+ * (eg. DISPLAY isn't set this will hang... */
down(&data->sem);

free_irq_by_irq_and_dev(XTERM_IRQ, data);


2004-11-15 17:22:52

by Blaisorblade

[permalink] [raw]
Subject: Re: [uml-devel] [PATCH] uml: fail xterm_open when we have no $DISPLAY

On Monday 15 November 2004 04:25, Chris Wedgwood wrote:
> If UML wants to open an xterm channel and the xterm does not run
> properly (eg. terminates soon after starting) we will get a hang.
> This avoids the most common cause for this and adds a comment (which
> long term will go away with a rewrite of that code I guess?)

Thanks for drawing attention on this! I hadn't had the time to dig this out...
for now reasonably correct, but no time to review it properly now. The review
would probably try to find a more direct fix to this...
--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729

2004-11-16 06:42:44

by Chris Wedgwood

[permalink] [raw]
Subject: Re: [uml-devel] [PATCH] uml: fail xterm_open when we have no $DISPLAY

On Mon, Nov 15, 2004 at 06:24:29PM +0100, Blaisorblade wrote:

> Thanks for drawing attention on this! I hadn't had the time to dig
> this out... for now reasonably correct

it's a partial solution, in general more is required

> but no time to review it properly now. The review would probably try
> to find a more direct fix to this...

catch the xterm process dying, up(&data->sem), and -EIO all requests
from that point onwards I guess. That applies for some of the other
channels too, some of the code would probably be abstracted a little
and generalized I guess.



--cw