>> >> Why not change the logic and instead of not closing when there is no >> iochannel, we just remove the g_io_channel_shutdown() calls? We know >> that g_io_channel_unref() won't do any implicit close, so we can >> safely destroy the iochannel regardless of whether we leave the >> underlying FD open, and then close the FD with close() ourselves >> unconditionally. That would probably be easier to understand as well? >> Instead of the shutdown() call we could place a big fat /* comment */ >> explaining why we don't call shutdown. > >> What do you think? >> > > If we directly close() instead of using g_io_channel_shutdown() then > (1) the channel buffer won't be flushed before closing (which is > probably fine?), and
Isn't there an explicit buffer flush using the fd when closing the channel just a bit before? I believe I saw that when I checked the code. > (2) if there are other refs to the channel and the "closing" > g_io_channel_unref() doesn't destroy it, then the channel's internal > record keeping will think that the closed fd is still open and > readable/writable. > I know (2) is not a problem today because there are no other > g_io_channel_ref() calls, but it could be overlooked if one was added in > future. > > Perhaps a better way would be to only use g_io_channel_shutdown() and > replace priv->fd with g_io_channel_unix_get_fd() where required for the > tcgetattr/tcsetattr/ioctl? > Removing priv->fd would mean the fd is tracked in only one place. The > "self->priv->fd >= 0" checks would become the "self->priv->iochannel" > check so the fd being open would be tied directly to the iochannel being > present. > I also thought about that yesterday, but then the shutdown removal seemed like a very quick and simple solution for the issue anyway. If you want to suggest a patch with the change to use g_io_channel_unix_get_fd() instead of self->priv->fd, I'll gladly review it, though. :) -- Aleksander https://aleksander.es _______________________________________________ ModemManager-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
