[PATCH] port-serial: don't close tty file descriptor twice

[Aleksander Morgado]

>>
>> 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