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

View Change

8 comments:

• Patchset:

  • Patch Set #5:

    > > Since we already have osmo_stream_*_get_ofd() we should add osmo_stream_*_get_iofd(). […]

    I see no problem with having separate APIs regarding what you mention. The socket may be already enabled, but it may not trigger any callback until you return to the mainloop, so you can perfectly call the APIs to set the callbacks before they are called.

• File src/stream.c:

  • Patch Set #7, Line 554: static void handle_connecting(struct osmo_stream_cli *cli, int res)

    this is for osmo_stream_cli, so adding a "stream_cli" prefix would help easily gastp that.

  • Patch Set #7, Line 598: handle_connecting(cli, res);

    This split into a separate function you could do it in a previous separate preparation patch.

  • Patch Set #7, Line 620: if (msg && res <= 0) {

    EAGAIN would trigger closing the socket?

  • Patch Set #7, Line 881: cli->ofd.priv_nr = 0; /* XXX */

    what about this XXX?

  • Patch Set #7, Line 1046: cli->iofd = osmo_iofd_setup(cli, -1, cli->name, OSMO_IO_FD_MODE_READ_WRITE, &osmo_stream_cli_ioops, cli);

    shouldn't this -1 be "fd"?

  • Patch Set #7, Line 1540: osmo_stream_srv_destroy(conn);

    just wondering whether you may need to return -EBADF here?

  • Patch Set #7, Line 1714: conn->iofd_read_cb = read_cb;

    See, these can perfectly be separate setter APIs like we do for osmo_stream_cli.

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