Attention is currently required from: arehbein, dexter, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34444?usp=email )
Change subject: write_queue: Enable updating max_length field ...................................................................... Patch Set 1: Code-Review-1 (1 comment) File src/core/write_queue.c: https://gerrit.osmocom.org/c/libosmocore/+/34444/comment/da9d2090_f795f7c1 PS1, Line 163: dropped_msgs < diff Imagine the following situation: the user wants to reduce the queue limit (`max_length`) from 128 to 64. The queue contains 32 items (`current_length`). ``` int diff = queue->max_length - len; // 128 - 64 == 64 queue->max_length = len; // 64 for (dropped_msgs = 0; dropped_msgs < 64 && !llist_empty(q); dropped_msgs++) ``` so IIUC, this function would dequeue and free these 32 items, despite their count does not exceed the new limit (64). This looks wrong to me. Another problem is that you're not updating the `current_length` in this function at all. I suggest the following: ``` while (queue->current_length > queue->max_length) { msg = msgb_dequeue_count(&queue->msg_queue, &queue->current_length); msgb_free(msg); dropped_msgs++; } ``` -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34444?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 34444 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein <arehb...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pma...@sysmocom.de> Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-Attention: arehbein <arehb...@sysmocom.de> Gerrit-Attention: pespin <pes...@sysmocom.de> Gerrit-Attention: dexter <pma...@sysmocom.de> Gerrit-Comment-Date: Sun, 17 Sep 2023 14:35:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
