Attention is currently required from: arehbein, daniel, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email )
Change subject: meas_feed: Use osmo_io instead of write queue
......................................................................
Patch Set 5: Code-Review-1
(2 comments)
File src/osmo-bsc/meas_feed.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/c0b96c80_692b4958
PS5, Line 148: &(struct osmo_io_ops) {
: .read_cb = meas_feed_noop_cb,
: .write_cb = meas_feed_noop_cb,
: .segmentation_cb = NULL,
: }, NULL);
Can we have this struct defined outside of the function call?
IMO, passing a struct pointer like this is not so readable.
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/b998a2e0_83fb04f5
PS5, Line 156: meas_feed_txqueue_max_length_set(MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT
This looks suspicious. I see a few potential problems:
* Scenario a): in the config file `meas-feed write-queue-max-length <1-65535>` goes before `meas-feed destination ADDR <0-65535>`: in this case you're overwriting the user's setting with `MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT`.
* Scenario b): a user willing to change the destination addr/port entering the VTY and issuing the `meas-feed destination ADDR <0-65535>` command: in this case you're again overwriting the user's setting with `MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT`.
I think the correct way would be to call:
```
osmo_iofd_set_txqueue_max_length(g_mfs.io_fd, g_mfs.max_length);
```
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ib0570a3242e2846062e24c93cbbbbd31137acdee
Gerrit-Change-Number: 34526
Gerrit-PatchSet: 5
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 02 Oct 2023 08:29:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: daniel, pespin.
arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email )
Change subject: meas_feed: Use osmo_io instead of write queue
......................................................................
Patch Set 5:
(3 comments)
This change is ready for review.
Patchset:
PS1:
> Build tests are passing now; there are some TODO-marked comments that may need clarification/I still […]
Done
Patchset:
PS4:
> I agree that renaming the VTY command in particular is unnecessary and will cause more fallout than […]
Done
File src/osmo-bsc/meas_feed.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/8f9d33e8_f83bf043
PS4, Line 117: if (g_mfs.io_fd == NULL) /* TODO: Check if this is the right condition to check if io_fd is initialized */
> ah yeah. Pushing an update to the patch unset the WIP flag that I originally pushed the patch with. […]
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ib0570a3242e2846062e24c93cbbbbd31137acdee
Gerrit-Change-Number: 34526
Gerrit-PatchSet: 5
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Sun, 01 Oct 2023 22:36:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment