hi holger,
i reworked the patch.
Holger Hans Peter Freyther wrote:
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
alternatively the create and rtp_sub_socket_create and rtp_sub_socket_close. this makes more work, but i was lazy.
+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.
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