Attention is currently required from: arehbein.
9 comments:
File src/stream.c:
I'd go for EINVAL here. It's not like there are any other modes in the enum OSMO_STREAM_MODE which we don't support. Rather, we are sure that whatever other value is passed here is invalid.
I'd usually say that it is "established" after connect succeeds?
Patch Set #1, Line 654: error %d to send
sounds a bit like we are "about to send ... as an error". "received error %d in response to send" (if that's the meaning intended)?
Patch Set #1, Line 683: osmo_iofd_setup
osmo_iofd_setup may return NULL. missing error handling.
Patch Set #1, Line 1715: LOGL_DEBUG
this message might be worth INFO or even NOTICE, I think.
Patch Set #1, Line 1758: OSMO_ASSERT
why is this commented out? Is the idea to permit a srv_link in ofd mode and the stream_srv in osmo_io mode? It would be an exotic combination, but if it's technically possible, then ok. Either way the assret should either be active or removed.
Patch Set #1, Line 1762: LOGP(DLINP, LOGL_ER
if the memory allocator fails, it's always an ENOMEM situation, so it's not realyl worth calling strerror or printing something about the reason. Also, we're typically not adding log messages in such situations as otherwise we would have to write a gadzillion of them every 5th line of code. If one wanted to log this, it would be best done inside talloc or in a wrapper around talloc. That way it's one log statement for all situations.
Patch Set #1, Line 1768: osmo_iofd_setup
osmo_iofd_setup may return NULL. Missing error handling
Patch Set #1, Line 1775: talloc_free
is there nothing we need to call to undo whatever osmo_iofd_setup() has done above?
To view, visit change 33193. To unsubscribe, or for help writing mail filters, visit settings.