Attention is currently required from: arehbein.
Patch set 2:Code-Review -1
7 comments:
File include/osmo-bts/pcuif_proto.h:
Patch Set #2, Line 8: #define PCU_SOCK_QLENGTH_MAX_DEFAULT 10
that's application/peer implementation specific, not really part of the shared protocol and hence shouldn't be placed here.
Bear in mind this file is duplicated in 3 different project, so avoid adding protocol-unneeded stuff in here.
File src/common/pcu_sock.c:
Patch Set #2, Line 988: if (osmo_wqueue_enqueue(&state->upqueue, msg) == -ENOSPC) {
Simply check for any error "< 0"
Patch Set #2, Line 989: LOGP(DLGLOBAL, LOGL_NOTICE, "PCU not reacting (more thatn %d messages waiting). Closing connection\n",
max_length is an int? or an unsigned?
Patch Set #2, Line 992: } else {
Usually we do early return on error, then drop this "else" block.
Patch Set #2, Line 1092: return -1;
In heere probably -EBADF needs to be returned so that the wqueue potentially calls the write_cb() with a closed/freed socket.
Patch Set #2, Line 1102: osmo_fd_write_disable(&state->upqueue.bfd);
This is wrong. You should disable the bfd after successful write() only if the queue is empty.
Patch Set #2, Line 1181: osmo_wqueue_init(&state->upqueue, PCU_SOCK_QLENGTH_MAX_DEFAULT);
I would split this into 2 commits if possible:
1 commit changing the implementation to wqueue but keeping unlimited wqueue size (is that possible by passin 0 here?)
1 commit afterwards changing the queue size to PCU_SOCK_QLENGTH_MAX_DEFAULT.
btw, PCU_SOCK_WQUEUE_LEN it's more clear imho (had to think hat QLENGTH meant).
To view, visit change 31533. To unsubscribe, or for help writing mail filters, visit settings.