Attention is currently required from: arehbein.

Patch set 2:Code-Review -1

View Change

1 comment:

• Patchset:

  • Patch Set #2:

    Re-adding the comments from the private change:

    daniel:
    Can you elaborate on that? Right now the expectation is that the user calls write_enable separately. I guess we could change the behaviour if we never want to enqueue messages that should not be sent immediately.

    In that case we should also change the sendmsg function below to behave the same.

    arehbein:
    A couple of reasons which together strongly suggested to me it should be like this:

    • osmo_stream_srv_send() was broken because the 'write enable' was missing
    • The doc for osmo_iofd_write_msgb() says /*! Send a message through a connected socket
    • The function name suggests that the write should be caused by it.

    Maybe the last two points are also why the write enable was omitted in osmo_stream_srv_send() .

    Admittedly, I didn't check the equivalent *_cli_* function...
    however I'm in favor of renaming the function ( osmo_iofd_write_msgb -> osmo_iofd_append_msgb maybe?) and fixing the doc if we stick with the current approach/the one you are mentioning.

    I can see that it's confusing as it is right now. Maybe the issue is that we currently require a call to read/write_enable when opening the io_fd.
    Instead it should probably be the default. If the user calls read/write_disable it's much more obvious that they'll need to call *_enable again.
    

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