bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: patch to add error checking in term


From: Roland McGrath
Subject: Re: patch to add error checking in term
Date: Sat, 2 Feb 2002 20:13:14 -0500 (EST)

> this patch changes the interfaces of the bottom handler to return error
> values.

Cool.

> To keep it simple, for now every old function is changed to return 0.
> The actual error checking in devio.c for example can then be added later.

For devio we should examine what the drivers do and think about the details.
There are probably lots of kinds of errors from drivers not supporting
things fully that we should ignore in the devio layer.

> This is also desirable because I am not sure if term can really cope with
> these functions returning errors in all places, so I want to keep it
> incremental (the hurdio bottom handler will return errors, and then we
> can test it there first, before we go back and add error checking to the
> device bottom handler).

Sounds good.

> In general, some problems stem from the fact that term does some global
> diddling, then calls the bottom handler and expects it to cope.  

I think we should change the functions like set_bits that now look at
global variables to instead take arguments, with the protocol that if the
function returns an error no change has happened.  Then make the callers
not change the global state until after they've called the bottomhalf and
seen a 0 return value.

> po_destroy_hook: Clearly error checking is useless here.  We can only hope
>                  for the best.

Indeed.  There is nothing that needs to be done.

> trivfs_S_io_write: I have added error checking, but there is one additional
> start_output at the end of it, which is called unconditionally.  We should
> probably conditionalize this (and ignore its error), any suggestions? 

The only errors you can get are from start_output.  If a start_output call
just failed, there is no sense in calling it again immediately.  There is
also no reason for it to call start_output if datalen was 0.  In both
cases, I think you should ignore the start_output error if any chars were
queued (i.e. i>0) and report it if none were (i.e. i==0).  

> S_tiocltl_tiocflush, set_state:
> We clear the queue before notifying the bottom handler of the flush (or
> abandoning all output).  Does it make sense to ask the bottom handler to
> flush, and if that fails, not to clear our queue?

Yeah, maybe.  That is by the theory that if you can't do it all, you
propagate the error up and leave nothing changed.  There is no extra cost
to doing it that way.

> set_state:  I don't know if we can recover from a failed set_bits, as the
> global termstate is edited.  Can someone check this out?  Maybe saving and
> restoring the old state.

The set_bits interface I described above would make this more clear.

> munge.c:
> I did not make any changes here.  The only change that we could make is
> to check the return value of suspend_physical_output and only diddle the bit
> if that worked.  For start_output I don't think we can do anything useful on
> failure, and for abandon_physical_output see above (clearing the queue). 
> drop_output is a void function anyway.

But drop_output could return error, and that would be the right thing for
the S_tioctl_tiocflush behavior described above.  The calls in munge.c
should ignore errors from drop_output.

Let's figure out these details and get a more complete set of changes
before you put this in.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]