Change in libosmocore[master]: gprs_ns2: dont use llist_for_each when freeing an element

dexter gerrit-no-reply at lists.osmocom.org
Mon Aug 9 12:01:49 UTC 2021


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

Change subject: gprs_ns2: dont use llist_for_each when freeing an element
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

The new way of freeing all the nsvc elements looks a bit strange to me.

I think the event will not be processed while llist_for_each() is running, but maybe in the next select loop cycle.

In any case, I think this patch would not break anything but I do not really understand what it is fixing exactly.

https://gerrit.osmocom.org/c/libosmocore/+/25147/1/src/gb/gprs_ns2_sns.c 
File src/gb/gprs_ns2_sns.c:

https://gerrit.osmocom.org/c/libosmocore/+/25147/1/src/gb/gprs_ns2_sns.c@1584 
PS1, Line 1584: 		while (!llist_empty(&nse->nsvc)) {
is this because gprs_ns2_free_nsvc() may free multiple nsvc from the list so that it is impossible to iterate the list normally? You always check if the list is empty, then you get the first entry to free it and then again. A comment why this has to be handled this way might be helpful.



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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I902557fb6e56e6588728a46e43a9cbe3215d5c68
Gerrit-Change-Number: 25147
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-CC: laforge <laforge at osmocom.org>
Gerrit-Comment-Date: Mon, 09 Aug 2021 12:01:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210809/123018d1/attachment.htm>


More information about the gerrit-log mailing list