pespin has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-bts/+/31924 )
Change subject: Rewrite pcu_sock_write()
......................................................................
Rewrite pcu_sock_write()
The code in that function is pretty rotten:
* osmo_fd_write_disable() is called for each message in the queue,
there's no need for that. Let's simply call it at the end if the queue
is empty.
* Asserting for obvious stuff like dequeue returning the first entry in
the list.
* Having error code path for empty message: That shouldn't happen, abort
immediately.
With all thse changes, the function is way simpler, easy to read and
more efficient.
Change-Id: I7ffff98cd8cdf2041bff486c3fde6f16365028d5
---
M src/common/pcu_sock.c
1 file changed, 31 insertions(+), 25 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/24/31924/1
diff --git a/src/common/pcu_sock.c b/src/common/pcu_sock.c
index 145942b..b639026 100644
--- a/src/common/pcu_sock.c
+++ b/src/common/pcu_sock.c
@@ -28,6 +28,7 @@
#include <inttypes.h>
#include <osmocom/core/talloc.h>
+#include <osmocom/core/utils.h>
#include <osmocom/core/select.h>
#include <osmocom/core/socket.h>
#include <osmocom/gsm/gsm23003.h>
@@ -1087,48 +1088,32 @@
static int pcu_sock_write(struct osmo_fd *bfd)
{
struct pcu_sock_state *state = bfd->data;
+ struct msgb *msg;
int rc;
- while (!llist_empty(&state->upqueue)) {
- struct msgb *msg, *msg2;
- struct gsm_pcu_if *pcu_prim;
-
- /* peek at the beginning of the queue */
- msg = llist_entry(state->upqueue.next, struct msgb, list);
- pcu_prim = (struct gsm_pcu_if *)msg->data;
-
- osmo_fd_write_disable(bfd);
-
+ while ((msg = msgb_dequeue(&state->upqueue))) {
/* bug hunter 8-): maybe someone forgot msgb_put(...) ? */
- if (!msgb_length(msg)) {
- LOGP(DPCU, LOGL_ERROR, "message type (%d) with ZERO "
- "bytes!\n", pcu_prim->msg_type);
- goto dontsend;
- }
+ OSMO_ASSERT(msgb_length(msg) > 0);
/* try to send it over the socket */
rc = write(bfd->fd, msgb_data(msg), msgb_length(msg));
- if (rc == 0)
+ if (OSMO_UNLIKELY(rc == 0))
goto close;
- if (rc < 0) {
+ if (OSMO_UNLIKELY(rc < 0)) {
if (errno == EAGAIN) {
- osmo_fd_write_enable(bfd);
- break;
+ msgb_enqueue(&state->upqueue, msg);
+ return 0;
}
goto close;
}
-
-dontsend:
- /* _after_ we send it, we can deueue */
- msg2 = msgb_dequeue(&state->upqueue);
- assert(msg == msg2);
msgb_free(msg);
}
+ osmo_fd_write_disable(bfd);
return 0;
close:
+ msgb_free(msg);
pcu_sock_close(state);
-
return -1;
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31924
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I7ffff98cd8cdf2041bff486c3fde6f16365028d5
Gerrit-Change-Number: 31924
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <[email protected]>
Gerrit-MessageType: newchange
