Change in libosmocore[master]: Add inter-thread queue

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

pespin gerrit-no-reply at lists.osmocom.org
Mon Jan 4 11:12:11 UTC 2021


pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/21930 )

Change subject: Add inter-thread queue
......................................................................


Patch Set 2:

(10 comments)

https://gerrit.osmocom.org/c/libosmocore/+/21930/2/include/osmocom/core/it_q.h 
File include/osmocom/core/it_q.h:

https://gerrit.osmocom.org/c/libosmocore/+/21930/2/include/osmocom/core/it_q.h@37 
PS2, Line 37: 	void (*read_cb)(struct osmo_it_q *q, void *item);
why not passing the pointer to the item's llist instead? This way it's up to the reader to infer item form llist using well-known container_of().
This way you avoid the limitation of requiring llit at the beggining of item's struct. (see like 17 "IMPORTANT").


https://gerrit.osmocom.org/c/libosmocore/+/21930/2/src/it_q.c 
File src/it_q.c:

https://gerrit.osmocom.org/c/libosmocore/+/21930/2/src/it_q.c@67 
PS2, Line 67: /* global (for all threads) list of message queues in a program + associated lock */
typo: queued


https://gerrit.osmocom.org/c/libosmocore/+/21930/2/src/it_q.c@69 
PS2, Line 69: static pthread_rwlock_t it_queues_rwlock;
you can probably init this statically?


https://gerrit.osmocom.org/c/libosmocore/+/21930/2/src/it_q.c@113 
PS2, Line 113: 		q->read_cb(q, item);
See, simply change function signature to pass struct llist_head and let the user implementing the callback to use container_of.


https://gerrit.osmocom.org/c/libosmocore/+/21930/2/src/it_q.c@142 
PS2, Line 142: 	q->event_ofd.fd = -1;
q->mutex needs no init()?


https://gerrit.osmocom.org/c/libosmocore/+/21930/2/src/it_q.c@220 
PS2, Line 220: 	osmo_it_q_flush(q);
don't you need to somehow free q->mutex?


https://gerrit.osmocom.org/c/libosmocore/+/21930/2/tests/it_q/it_q_test.c 
File tests/it_q/it_q_test.c:

https://gerrit.osmocom.org/c/libosmocore/+/21930/2/tests/it_q/it_q_test.c@10 
PS2, Line 10: 	int foo;
You could put a pointer to g_read_cb_count here to have better coverage on struct being correctly used instead of simply allocating and freeing it.


https://gerrit.osmocom.org/c/libosmocore/+/21930/2/tests/it_q/it_q_test.c@73 
PS2, Line 73: static void q_read_cb(struct osmo_it_q *q, void *item)
struct llist_head *item


https://gerrit.osmocom.org/c/libosmocore/+/21930/2/tests/it_q/it_q_test.c@75 
PS2, Line 75: 	g_read_cb_count++;
struct it_q_test1 *it = container_of(item, struct it_q_test1, list);
(*it->foo)++


https://gerrit.osmocom.org/c/libosmocore/+/21930/2/tests/it_q/it_q_test.c@100 
PS2, Line 100: 		item = talloc_zero(OTC_GLOBAL, struct it_q_test1);
item->foo = &g_read_cb_count;



-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/21930
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ie7d0c5fec715a2a577fae014b0b8a0e9c38418ef
Gerrit-Change-Number: 21930
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann at sysmocom.de>
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Mon, 04 Jan 2021 11:12:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210104/071e57f1/attachment.htm>


More information about the gerrit-log mailing list