Hi again Denis,
On 29/04/2011 15:12, Guillaume Zajac wrote:
Hi Denis,
On 28/04/2011 21:41, Denis Kenzior wrote:
Hi Guillaume,
+ /* create a channel for reading and writing to this
interface */
+ channel = g_io_channel_unix_new(fd);
+ }
There's a small problem of symmetry here. If IPCP is established
correctly, then the fd will eventually be closed by ppp_net.
However,
if we never properly establish IPCP, then fd will not be closed.
What
is actually expected by ConnMan?
In the case we don't properly establish the IPCP, ppp_disconnect
CB of
emulator will be called.
Then, we will call the release_private_network DBus method. This
method
will close the fd if it is opened.
Again, we're writing a library. We cannot assume some behavior
external
to the library is going to save us. So I really suggest we make
closing
of the fd symmetric.
So I will close the fd into g_at_ppp_unref() if it is>=0.
I have also to close it if we fail to create the PPP server.
Right. Your approach doesn't sound completely correct, but I will let
you figure this out. Just make sure to close the fd in all
circumstances.
In looking the previous implementation in ppp_net_new(),
there is just:
fd = open("/dev/net/tun", ...);
...
net->channel = g_io_channel_unix_new(fd);
However, the fd is never closed when we do ppp_net_free()
We just unref the net->channel
So should we do in ppp_net_free() a:
fd = g_io_channel_unix_get_fd(net->channel);
close(fd);
g_io_channel_unref(net->channel);
Or maybe I miss something?
In fact I missed something :) , the g_io_channel_unref() is closing fd
In the new implementation, in the case we don't establish the IPCP, we
still have the fd of ConnMan opened.
ppp->net == NULL so in the g_at_ppp_unref() we can do:
if (ppp->net)
ppp_net_free(ppp->net); /* We close the fd like above */
We close it during unref.
else {
if( fd >= 0)
close(ppp->fd); /* We don't have established IPCP, we
need to close the fd from ConnMan */
}
We might need also to do:
ppp->fd = -1;
into ppp_ipcp_down_notify() just after ppp_net_free()
Kind regards,
Guillaume
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
