This stuff all looks great. As usual, Marcus, you rule. It would be ideal to separate the commits into: bottomhalf interface changes, using argp, and adding the hurdio bottomhalf. But if you don't want to bother then I don't really care. But please do clean up the log entries not to make reference to intermediate states and changes from intermediate states along the way, just describe the the changes from the existing code to the code you are checking in with each batch described by a log entry.
I haven't really scoured the code in detail, but if it works cleanly for you we can get it in now and change nits later as I notice them in reading over the code. Something I realized is that before your changes the CIGNORE behavior was totally wrong. When CIGNORE is given in a set_state call (TIOCSETA*), the c_cflag and c_*speed fields should not be used at all, and the bottomhalf set_bits hook should never even be called. This is consistent with the BSD implementation (Linux does not have CIGNORE). The existing Hurd behavior of clobbering termstate is wrong. In general, we should check for anything like this in the code. For anything that can have an error, the global state should not be tweaked before the bottomhalf call is attempted, and the bottomhalf call should have an argument for the new state being put in. Only in the call succeeds should term's state variables be updated. Maybe you have covered this already. Adding the init hook is a good thing. Off-hand nits: Please always use "type\nfunction (...)" instead of "type function". devio_init can be static. mdmstate should return error_t and take an int * argument to fill in. The underlying access can fail and then it should not necessarily just return 0. e.g. for hurdio all things that boil down to tioctl calls ought to ignore them if they return EOPNOTSUPP/MIG_BAD_ID/ENOSYS but propagate other errors to their callers (EIO or whatever it might be). It never hurts to do bcopy -> memcpy while you're there if you feel like it. Use -p to diff. For get_bottom_type, let's just add an enum term_bottom_type field to bottomhalf. The struct bottomhalfs ought to be declared const. At some point I would like to move the bottomhalf's and the rest of term away from using global variables. The termflags et al should move into a state structure that the trivfs hook will point to. For the bottomhalf's, we could have either a union or an allocated void * in there for their private data, and have the state pointer passed in all the hooks. (All of this is just like storeio's struct dev.) As well as plain style, this gets us closer to being able to use fsys_delegate for multiple (root-owned) terminals to share a process for the term translator. It might be good to have a stub bottomhalf (similar to the "unknown" store type I added), where every hook is just a no-op or returns EIO. It's not good for much, but it's handy for testing sometimes and you can give it an option where it prints out for every hook call or something. Putting that underneath the hurdio bottomhalf is a good way to test that every termios operation is passing down in the proper fashion. (Also I figure if we did implement fsys_delegate, then /servers/term would be a term node with a stub bottomhalf so the only thing it's good for is stat or delegate.) You talked about using FIFOs and that seems pretty good for debugging purposes (sorry it didn't occur to me in the first place you wouldn't have been using two of them or I would have forewarned you on that). But we can't necessarily always trust pflocal when suspicious things are happening, and it made me think that it would be most useful to write a simple translator to be your own test harness. That is, a trivfs filesystem where io_read just iterates through a list of canned test data to reply with. Or you could make it like a modified streamio but rather than a device-reading thread you have a thread driven by a list of delays and characters to deliver (if you get that fancy, you can have it read the test script from a file and make it flexible what loads you drive it with). But the real benefit over just FIFOs or a plain file for testing purposes is that you can implement the various tioctl calls to work or fail in interesting ways that you want to test from above. If you are on random feature patrol, seems like we might as well give term the --readonly/--writeonly options (and --writable and all aliases) streamio has. I don't know why you'd want to use them in term, but then I don't know off hand why you'd want them in streamio either. :) I guess we might as well have --rdev too. _______________________________________________ Bug-hurd mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/bug-hurd
