Attention is currently required from: arehbein, daniel, pespin.

Patch set 5:Code-Review -1

View Change

2 comments:

• File src/osmo-bsc/meas_feed.c:

  • Patch Set #5, 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.

  • Patch Set #5, 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 change 34526. To unsubscribe, or for help writing mail filters, visit settings.