Refactor: Refactor the mainrelay.c #1301
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KangLin
force-pushed
the
refactor
branch
12 times, most recently
from
7d744fb
to
36f5051
Compare
KangLin
force-pushed
the
refactor
branch
7 times, most recently
from
70c51a8
to
6896d6c
Compare
|
@SamuelMarks @eakraly @rolandwu777 @ggarber @pando-emil Please review. |
KangLin
force-pushed
the
refactor
branch
3 times, most recently
from
2ed5784
to
3609579
Compare
KangLin
force-pushed
the
refactor
branch
2 times, most recently
from
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{}.
jonesmz
reviewed
Jan 19, 2024
| 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 <kl222@126.com> | ||
| // Because of the fd is noblock socket |
There was a problem hiding this comment.
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.
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.
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.
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.
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.
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.
erm.... this is not a great name for a variable...?
especially since it seems this specific variable does not accomplish anything.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor the mainrelay.c for add windows service.
See #1300