Hi,

Thank you!

On Wed, Dec 13, 2023 at 2:35 PM Ondrej Zajicek <santi...@crfreenet.org> wrote:
>
> On Wed, Dec 13, 2023 at 11:50:42AM +0100, Alexander Zubkov wrote:
> > Hi,
> >
> > Thanks! I looked throught your version and it is unclear to me if the
> > sk is still added to the io loop list (sock_list) or not. It seems
> > that sk_insert() still should be called on log udp socket, because I
> > see no exception for it.
>
> Hi
>
> I use SKF_THREAD (like in your patch), so it is not inserted intoo sock_list.

Huh, now I see it clearly. :) Oversaw it, sorry.

>
> > And could you, please, also explain another moment:
> > https://gitlab.nic.cz/labs/bird/-/commit/2c7555cf2ac8439713dd9148b348128c57222a38#bc490dc797778621d2345fabe1fb0b59fce18264_141_179
> > Here your free the sk resource. Maybe this is done exactly because of
> > the io loop list, but I cannot find how sk_free() would remove the
> > socket from sock_list. It seems to me it would still refer the socket
> > after it is freed.
>
> sk_free() calls rem_node(&s->n) for non-SKF_THREAD sockets, so it would
> remove it from sock_list anyway.

As it is SKF_THREAD, it is not relevant yet. But I see here fd is set
to -1 before rfree():
https://gitlab.nic.cz/labs/bird/-/commit/2c7555cf2ac8439713dd9148b348128c57222a38#bc490dc797778621d2345fabe1fb0b59fce18264_141_179
And sk_free() will not call rem_node() in this case:
https://gitlab.nic.cz/labs/bird/-/blob/2c7555cf2ac8439713dd9148b348128c57222a38/sysdep/unix/io.c#L834
I worry it can lead to a hidden error in the future.

>
> The socket structure is freed because it is no longer needed,
> as the fd is tracked by the rfile structure anyway.

Yes, it seems OK. I just wasn't sure it is cleared properly.

>
> --
> Elen sila lumenn' omentielvo
>
> Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org)
> "To err is human -- to blame it on a computer is even more so."

Reply via email to