Attention is currently required from: osmith, Hoernchen, neels, laforge, 
pespin, fixeria.

daniel has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/30934 )

Change subject: Add osmo_io with initial poll backend
......................................................................


Patch Set 13:

(4 comments)

Patchset:

PS13:
Sorry for the back and forth changes, I tried to merge your changes and still 
keep the lost changes from 
https://gerrit.osmocom.org/c/libosmocore/+/30934/8..9 and I had some trouble 
seeing what was intentional and what was merely code from a previous version.


File src/core/osmo_io.c:

https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/fcdac5d7_ddc8be5c
PS8, Line 206:  iofd->tx_queue.current_length--;
> Could be msgb_dequeue_count(), but can stay this way too.
See discusion in iofd_txqueue_enqueue()


File src/core/osmo_io.c:

https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/e883e478_f7a2eaf4
PS13, Line 82:          fprintf(stderr, "Invalid osmo_io backend requested: 
\"%s\nCheck the environment variable %s\n", backend, OSMO_IO_BACKEND_ENV);
> You are missing a \" before first \n.
Thanks, fixed


File src/core/osmo_io_poll.c:

https://gerrit.osmocom.org/c/libosmocore/+/30934/comment/2d358e27_50d3d0df
PS13, Line 93:                  if (rc < 0) {
> I think I changed this and you are changing it again. […]
I though I had msg == NULL for a reason, but this is the sending side...

Thinking about it some more we definitely want to report a send rc of 0 to the 
user since that can indicate a broken connection. If we re-enqueue in that case 
we'll busy loop trying to send and send returning 0.

Keeping the msg does make sense so I'll change it to reenqueue only if (rc > 0 
&& rc < msgb_length(msg))



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30934
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I50d73cf550d6ce8154bf827bf47408131cf5b0a0
Gerrit-Change-Number: 30934
Gerrit-PatchSet: 13
Gerrit-Owner: daniel <dwillm...@sysmocom.de>
Gerrit-Reviewer: Hoernchen <ew...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de>
Gerrit-Reviewer: neels <nhofm...@sysmocom.de>
Gerrit-Reviewer: osmith <osm...@sysmocom.de>
Gerrit-Reviewer: pespin <pes...@sysmocom.de>
Gerrit-CC: laforge <lafo...@osmocom.org>
Gerrit-Attention: osmith <osm...@sysmocom.de>
Gerrit-Attention: Hoernchen <ew...@sysmocom.de>
Gerrit-Attention: neels <nhofm...@sysmocom.de>
Gerrit-Attention: laforge <lafo...@osmocom.org>
Gerrit-Attention: pespin <pes...@sysmocom.de>
Gerrit-Attention: fixeria <vyanits...@sysmocom.de>
Gerrit-Comment-Date: Fri, 05 May 2023 14:00:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pes...@sysmocom.de>
Gerrit-MessageType: comment

Reply via email to