[PATCH] libosmo-netif[master]: osmux: Add function to delete all msgs pending for a circuit
25 May
2016
25 May
'16
6:52 p.m.
Review at https://gerrit.osmocom.org/120
osmux: Add function to delete all msgs pending for a circuit
Use this function in osmux_batch_del_circuit() since msgs are stored in a list
per circuit. After the circuit is free()d the msgs are lost.
Before this patch any messages enqueued inside a batch when the circiut is
deleted were leaking.
Change-Id: Ib0311652183332d0475bf7347023d518d38487ef
Ticket: OS#1733
---
M src/osmux.c
1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/20/120/1
diff --git a/src/osmux.c b/src/osmux.c
index 44b4b96..01d0bdc 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -225,6 +225,17 @@
circuit->nmsgs--;
}
+static void osmux_circuit_del_msgs(struct osmux_batch *batch, struct osmux_circuit
*circuit)
+{
+ struct msgb *cur, *tmp;
+ llist_for_each_entry_safe(cur, tmp, &circuit->msg_list, list) {
+
+ osmux_batch_dequeue(cur, circuit);
+ msgb_free(cur);
+ batch->nmsgs--;
+ }
+}
+
struct osmux_input_state {
struct msgb *out_msg;
struct msgb *msg;
@@ -538,6 +549,7 @@
if (circuit->dummy)
batch->ndummy--;
llist_del(&circuit->head);
+ osmux_circuit_del_msgs(batch, circuit);
talloc_free(circuit);
}
--
To view, visit https://gerrit.osmocom.org/120
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib0311652183332d0475bf7347023d518d38487ef
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
25 May
25 May
9:28 p.m.
New subject: libosmo-netif[master]: osmux: Add function to delete all msgs pending for a circuit
Patch Set 1:
(1 comment)
https://gerrit.osmocom.org/#/c/120/1/src/osmux.c
File src/osmux.c:
Line 236: }
good. but indention is wrong. :)
--
To view, visit https://gerrit.osmocom.org/120
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0311652183332d0475bf7347023d518d38487ef
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Holger Freyther <holger(a)freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes
10:09 p.m.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/120
to look at the new patch set (#2).
osmux: Add function to delete all msgs pending for a circuit
Use this function in osmux_batch_del_circuit() since msgs are stored in a list
per circuit. After the circuit is free()d the msgs are lost.
Before this patch any messages enqueued inside a batch when the circiut is
deleted were leaking.
Change-Id: Ib0311652183332d0475bf7347023d518d38487ef
Ticket: OS#1733
---
M src/osmux.c
1 file changed, 11 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/20/120/2
diff --git a/src/osmux.c b/src/osmux.c
index 1f5bbe2..0bee9cc 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -225,6 +225,16 @@
circuit->nmsgs--;
}
+static void osmux_circuit_del_msgs(struct osmux_batch *batch, struct osmux_circuit
*circuit)
+{
+ struct msgb *cur, *tmp;
+ llist_for_each_entry_safe(cur, tmp, &circuit->msg_list, list) {
+ osmux_batch_dequeue(cur, circuit);
+ msgb_free(cur);
+ batch->nmsgs--;
+ }
+}
+
struct osmux_input_state {
struct msgb *out_msg;
struct msgb *msg;
@@ -538,6 +548,7 @@
if (circuit->dummy)
batch->ndummy--;
llist_del(&circuit->head);
+ osmux_circuit_del_msgs(batch, circuit);
talloc_free(circuit);
}
--
To view, visit https://gerrit.osmocom.org/120
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0311652183332d0475bf7347023d518d38487ef
Gerrit-PatchSet: 2
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Holger Freyther <holger(a)freyther.de>
Gerrit-Reviewer: Jenkins Builder
10:57 p.m.
New subject: libosmo-netif[master]: osmux: Add function to delete all msgs pending for a circuit
Patch Set 2:
(5 comments)
Okay, we can go through that code in terms of reliability. But you seem to fix it. The
timer question is not that important.
https://gerrit.osmocom.org/#/c/120/2/src/osmux.c
File src/osmux.c:
PS2, Line 234: batch
so what if nmsgs == 0? Will we need to stop the batch timer? What if the batch timer fires
and there is no single msgb? Did you review the code in that regard?
Line 325: uint32_t batch_size, uint32_t batch_factor)
How does it use the batch_factor? We seem to consume everything? Besides for the dummy
handling? Does it mean we put dummy + data into the same message?
Line 337: return NULL;
the caller will crash if that happens
PS2, Line 369:
Leaks the batch_msg. And discards other frames.. already formatted into the patch.. The
caller will crash.
Line 399: h->stats.output_osmux_bytes += batch_msg->len;
Unconditional access of batch_msg that might be NULL
--
To view, visit https://gerrit.osmocom.org/120
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0311652183332d0475bf7347023d518d38487ef
Gerrit-PatchSet: 2
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Holger Freyther <holger(a)freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes
26 May
26 May
2:05 p.m.
New subject: libosmo-netif[master]: osmux: Add function to delete all msgs pending for a circuit
Patch Set 2: Code-Review+2
Please update the tickets with the other things I found during review.
--
To view, visit https://gerrit.osmocom.org/120
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0311652183332d0475bf7347023d518d38487ef
Gerrit-PatchSet: 2
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Holger Freyther <holger(a)freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: No
2:05 p.m.
New subject: [MERGED] libosmo-netif[master]: osmux: Add function to delete all msgs pending for a circuit
Holger Freyther has submitted this change and it was merged.
Change subject: osmux: Add function to delete all msgs pending for a circuit
......................................................................
osmux: Add function to delete all msgs pending for a circuit
Use this function in osmux_batch_del_circuit() since msgs are stored in a list
per circuit. After the circuit is free()d the msgs are lost.
Before this patch any messages enqueued inside a batch when the circiut is
deleted were leaking.
Change-Id: Ib0311652183332d0475bf7347023d518d38487ef
Ticket: OS#1733
Reviewed-on: https://gerrit.osmocom.org/120
Tested-by: Jenkins Builder
Reviewed-by: Holger Freyther <holger(a)freyther.de>
---
M src/osmux.c
1 file changed, 11 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
Holger Freyther: Looks good to me, approved
diff --git a/src/osmux.c b/src/osmux.c
index 1f5bbe2..0bee9cc 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -225,6 +225,16 @@
circuit->nmsgs--;
}
+static void osmux_circuit_del_msgs(struct osmux_batch *batch, struct osmux_circuit
*circuit)
+{
+ struct msgb *cur, *tmp;
+ llist_for_each_entry_safe(cur, tmp, &circuit->msg_list, list) {
+ osmux_batch_dequeue(cur, circuit);
+ msgb_free(cur);
+ batch->nmsgs--;
+ }
+}
+
struct osmux_input_state {
struct msgb *out_msg;
struct msgb *msg;
@@ -538,6 +548,7 @@
if (circuit->dummy)
batch->ndummy--;
llist_del(&circuit->head);
+ osmux_circuit_del_msgs(batch, circuit);
talloc_free(circuit);
}
--
To view, visit https://gerrit.osmocom.org/120
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib0311652183332d0475bf7347023d518d38487ef
Gerrit-PatchSet: 3
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Holger Freyther <holger(a)freyther.de>
Gerrit-Reviewer: Jenkins Builder
3313
days inactive
3314
days old
5 comments
2 participants
participants (2)
-
daniel -
Holger Freyther