Created attachment 276597 [details] [review]
gsocket: Document FD ownership with g_socket_new_from_fd()

Are you sure?

It looks like if we fail during g_socket_details_from_fd() then we still have the fd in socket->priv->fd... we will then close it in finalize() when we destroy the half-created object.

(In reply to comment #2)
> Are you sure?
> 
> It looks like if we fail during g_socket_details_from_fd() then we still have
> the fd in socket->priv->fd... we will then close it in finalize() when we
> destroy the half-created object.

As I read it, the check_socket() call at the top of g_socket_close() will bail out because either:
 • !priv->inited; or
 • priv->construct_error != NULL
leaving the FD open.

(In reply to comment #3)
> As I read it, the check_socket() call at the top of g_socket_close() will bail
> out because either:
>  • !priv->inited; or
>  • priv->construct_error != NULL
> leaving the FD open.

Indeed... so the question now is whether that is a bug or not.

I guess there might be some code assuming the current behavior so it's probably best to preserve it?

(In reply to comment #4)
> (In reply to comment #3)
> > As I read it, the check_socket() call at the top of g_socket_close() will bail
> > out because either:
> >  • !priv->inited; or
> >  • priv->construct_error != NULL
> > leaving the FD open.
> 
> Indeed... so the question now is whether that is a bug or not.
> 
> I guess there might be some code assuming the current behavior so it's probably
> best to preserve it?

If I remember correctly I originally did a version of this patch which did close the FD, and I found that it was causing errors in a few places where existing code does assume the current behaviour. So I think we (unfortunately) do need to preserve it.

committed the docs patch to master and glib-2-44