29 Aug
2012
29 Aug
'12
8:22 a.m.
hi holger,
i reworked the patch.
Holger Hans Peter Freyther wrote:
alternatively the create and rtp_sub_socket_create and
rtp_sub_socket_close. this makes more work, but i was lazy.
The code is not dead. If bind of rtp socket failed to bind, rc is -1,
else if all sockets are tried, rc is -1, because the rtcp socket failed
to bind. if rc==0, both sockets are bound.
regards,
andreas
We had this before but I don't remember if it was
with the socket code. A
filedescriptor of '0' is valid. An unitiliazed bfd.fd should be set to -1.
But more importantly... why do we need to have these extra checks? The
code should only jump to the cleanup label that needs to be cleaned up.
So if the rtcp.bfd has not been registered, the code should not jump to
out_rtcp_bfd.
the checks were required, because rtp_socket_bind() might have closed a
socked. i removed the checks and closed both rtp and rtcp sockets at
rtp_socket_bind().
code duplication, please move the calculation to a
macro or inline function.
done.
+ init_rss(&rs->rtp, rs, rc2, RTP_PRIV_RTP);
+ rc2 = osmo_fd_register(&rs->rtp.bfd);
+ if (rc2 < 0) {
+ close(rs->rtp.bfd.fd);
+ return rc2;
+ }
Can you think of a way to have rtp_socket_create and this somehow share the
code to initialize the
+try_next_port:
+ next_udp_port = (next_udp_port + 2 > RTP_PORT_MAX) ?
+ RTP_PORT_BASE : next_udp_port + 2;
+ if (next_udp_port == start_port)
break;
+ /* we must use rc2, in order to preserve rc */
Hmm, maybe I deleted too much from the patch but break is only called
when rc == 0 and for all other errors you return immediately?
}
if (rc < 0) {
DEBUGPC(DLMUX, "failed\n");
I think this is dead code.