GNOME Bugzilla – Bug 333098
Add support for write FDs to GIOChannel
Last modified: 2006-03-11 21:03:27 UTC
Hi, this patch adds support for write fds to the w32 backend of GIOChannel. Currently, only read fds are supported. In GPGME, we need bidirectional communication to a child process, so we had to add this feature. The code was developed with the constraint that only minimal changes are made to the existing code base. This has caused a little bit of awkwardness: The same events as for the read thread are used, but with reversed meaning (ie, data_avail means space is availabel and vice versa). That's the only wart in the code that's specific to the write support. If you check out the code, ignoring the new functions write_thread and buffer_write, these are the changes to the generic code: A new field DIRECTION has been added to the channel structure, and in a couple of places the DIRECTION field or the condition flag (G_IO_IN/G_IO_OUT) is used to dispatch to the correct implementation. Also, the write function calls buffer_write if there is a writer thread now. This patch was developed as part of the GPG4Win project by g10 Code GmbH. If you have any questions, please don't hesitate to contact me. Thanks, Marcus Brinkmann mb@g10code.de
Created attachment 60468 [details] [review] Patch to add support for write FDs to GIOChannel on w32 targets
Thanks. Looks nice, don't see any reason not to accept and commit this. Later tonight. Will be in GLib 2.10.1.
Do you think there should be some check to catch if somebody for some reasons tries to watch a fd for both G_IO_IN and G_IO_OUT? There is just one thread_id field, will it confuse the code if first a G_IO_IN watch is created, then a G_IO_OUT (or vice versa)? Or should we have separate read_thread_id and write_thread_id fields?
Yes, I think there should be such a check. I didn't put it in because it can change the behaviour of existing code. Currently, with both flags set it will default to the reader implementation consistently. This is preserved by my patch. But yes, I think it would be a good idea to catch this case and bail out. Also, buffer_read and buffer_write should probably check the direction field if there is a thread running.
I forgot to respond to the separate thread ids issue. This is only necessary if you want to actually support waiting for both reading and writing, because then there will be both threads. Of course, then you also need separate events and buffers, etc. With a IN/OUT flag check, and the direction field, I think you can robustly catch all misuses of a channel object even with only one thread id field (as long as only unidirectional communication needs to be supported).
Thinking more about this, I guess it's pretty unlikely that anyone would want to have both a read and write watch for a normal file descriptor. That happens only for sockets. Applied the patch as such.