-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor: Refactor the mainrelay.c #1301
base: master
Are you sure you want to change the base?
Conversation
7d744fb
to
36f5051
Compare
70c51a8
to
6896d6c
Compare
@SamuelMarks @eakraly @rolandwu777 @ggarber @pando-emil Please review. |
2ed5784
to
3609579
Compare
e933782
to
66f5cc8
Compare
- add remove_listener() - add remove_socket_per_thread_udp_listener_servers() - add remove_socket_per_endpoint_udp_listener_servers() - add remove_socket_per_session_udp_listener_servers() - rename clean_server() to clean_dtls_listener_server()
- Remove exit() and free resource - Remove goto - Optimized "{}" multiple nesting, - Replace c98 (restrict variable declaration position at the beginning of the block) with c11 (no restriction variable declaration position in the block) - Optimized the multiple nesting and length of if{}.
relay_receive_auth_message() Using non-blocking improves thread performance
- Remove exit() and free resource - Remove goto - Optimized "{}" multiple nesting, - Replace c98 (restrict variable declaration position at the beginning of the block) with c11 (no restriction variable declaration position in the block) - Replace perror with TURN_LOG_FUNC
- Remove goto - Optimized "{}" multiple nesting, - Replace c98 (restrict variable declaration position at the beginning of the block) with c11 (no restriction variable declaration position in the block) Optimized the multiple nesting and length of if{}.
void read_spare_buffer(evutil_socket_t fd) { | ||
if (fd >= 0) { | ||
static char buffer[65536]; | ||
#if defined(WINDOWS) | ||
// TODO: add set no-block? by Kang Lin <[email protected]> | ||
// Because of the fd is noblock socket |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you reword this comment? it is not easy to read.
@@ -114,8 +101,7 @@ int set_sock_buf_size(evutil_socket_t fd, int sz0) { | |||
} | |||
|
|||
if (sz < 1) { | |||
perror("Cannot set socket rcv size"); | |||
TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "Cannot set rcv sock size %d on fd %d\n", sz0, fd); | |||
TURN_LOG_FUNC(TURN_LOG_LEVEL_INFO, "Cannot set rcv sock size %d on fd %d, err:%d\n", sz0, fd, socket_errno()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be nice if the socket_errno() was printed as a string and not (only) an integer.
@@ -114,8 +101,7 @@ int set_sock_buf_size(evutil_socket_t fd, int sz0) { | |||
} | |||
|
|||
if (sz < 1) { | |||
perror("Cannot set socket rcv size"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, if this function (set_sock_buf_size()
) takes as it's second argument a variable that'll be cast to socklen_t
then the parameter should be socklen_t
... not int
.
@@ -150,7 +135,7 @@ int socket_init(void) { | |||
/* Tell the user that we could not find a usable */ | |||
/* Winsock DLL. */ | |||
TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "WSAStartup failed with error: %d\n", e); | |||
return 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can the definition of wVersionRequested
be changed to also provide the initial value? Better than doing it in two steps.
same for int e
if (addr->ss.sa_family == AF_INET) { | ||
do { | ||
ret = bind(fd, (const struct sockaddr *)addr, sizeof(struct sockaddr_in)); | ||
} while (ret < 0 && socket_eintr()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you document what this loop is supposed to do?
why bind the same socket multiple times?
@@ -935,6 +940,8 @@ static ioa_engine_handle create_new_listener_engine(void) { | |||
&turn_params.redis_statsdb | |||
#endif | |||
); | |||
if (NULL == e) | |||
return e; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why return e
if e
is null?
@@ -943,73 +950,29 @@ static ioa_engine_handle create_new_listener_engine(void) { | |||
static void *run_udp_listener_thread(void *arg) { | |||
static int always_true = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
erm.... this is not a great name for a variable...?
especially since it seems this specific variable does not accomplish anything.
Refactor the mainrelay.c for add windows service.
See #1300