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

Patch set 6:Code-Review -1

View Change

3 comments:

• File src/common/pcu_sock.c:

  • Patch Set #6, Line 990: pcu_sock_close(state);

    I think you need to free msgb in this code path. […]

    Indeed, the API doc clearly states:

      \returns 0 on success; negative on error (MESSAGE NOT FREED IN CASE OF ERROR)

    so pespin is right here, please add `msgb_free(msg)`.

  • Patch Set #6, Line 1093: struct msgb *m

    I would keep the usual naming, i.e. `msg`. Not critical, but this way the patch/diff is easier to read.

  • Patch Set #6, Line 1106: rc = osmo_wqueue_enqueue(&state->upqueue, m);

    You mean at the beginning of the queue?

    Yes, this is likely what pespin meant. When this callback is called, it gets a msgb from the head (beginning) of the queue. But here you're putting it back at the tail, changing the ordering.

    Take a look at the `osmo_wqueue_bfd_cb()` in libosmocore.git. You don't really need to enqueue the msgb back yourself, just return `-EAGAIN` and the calling function will enqueue it back for you.

To view, visit change 31533. To unsubscribe, or for help writing mail filters, visit settings.