Attention is currently required from: arehbein, laforge, neels, pespin.
daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35079?usp=email )
Change subject: osmo_io: Remove union in struct osmo_io_ops
......................................................................
Patch Set 4:
(2 comments)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/35079/comment/1bcca390_cafaad48
PS3, Line 546: if (iofd->io_ops.read_cb)
> ah ok, read_cb happens to be in the same place as recvfrom_cb, so only checking the one location wor […]
This fix is only necessary after removing the union. I don't see how you could realize checking the exact member with the union in place. I can basically stick an
```c
OSMO_ASSERT(&iofd->io_ops.read_cb == &iofd->io_ops.recvfrom_cb)
```
in here. Unless I'm missing something this is exactly the feature of the union, afterwards you can't know whether this memory location was set as read_cb or recvfrom_cb.
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/35079/comment/813d11cc_3e337a84
PS4, Line 391: || (iofd->mode == OSMO_IO_FD_MODE_RECVFROM_SENDTO && iofd->io_ops.recvfrom_cb));
> this code should also work with the union present?
It "works" in the sense that it even returns true if mode == *_READ_WRITE and .recvfrom_cb (or the other way around) is set since both function pointers are sharing the same memory location.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35079?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I138d57843edc29000530bb7896bcb239002ecbec
Gerrit-Change-Number: 35079
Gerrit-PatchSet: 4
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 01 Dec 2023 10:05:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: falconia, fixeria, pespin.
jolly has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35132?usp=email )
Change subject: Transmit invalid AMR speech blocks instead of dummy FACCH
......................................................................
Patch Set 6:
(1 comment)
File src/osmo-bts-trx/sched_lchan_tchh.c:
https://gerrit.osmocom.org/c/osmo-bts/+/35132/comment/2caa0774_46c1a43c
PS6, Line 456: with
> Grammar still not fixed here - you fixed the one in TCH/F, but not here.
Grr....
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35132?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I056f379715c91ad968f198e112d363a9009dc1c3
Gerrit-Change-Number: 35132
Gerrit-PatchSet: 6
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 01 Dec 2023 10:02:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: falconia <falcon(a)freecalypso.org>
Gerrit-MessageType: comment
Attention is currently required from: fixeria, jolly, pespin.
Hello Jenkins Builder, falconia, fixeria, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/35132?usp=email
to look at the new patch set (#7).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: Transmit invalid AMR speech blocks instead of dummy FACCH
......................................................................
Transmit invalid AMR speech blocks instead of dummy FACCH
Every BTS needs to have some graceful handling for the scenario
where it is time to send out a speech frame on TCH DL, but there is
no frame to be sent. One possible solution is to transmit dummy
FACCH, but this option is unattractive for TCH/AHS where FACCH
displaces two speech frames rather than one. A more elegant solution
is to emit a speech frame that is bad, causing the MS receiver to
declare a BFI condition to trigger substitution and muting procedure.
A bad frame is generated by gsm0503_tch_{afs,ahs}_encode() by setting
the payload length to 0.
Depends: libosmocore.git I82ce2adf995a4b42d1f378c5819f88d773b9104a
Related: OS#6049
Change-Id: I056f379715c91ad968f198e112d363a9009dc1c3
---
M src/osmo-bts-trx/sched_lchan_tchf.c
M src/osmo-bts-trx/sched_lchan_tchh.c
2 files changed, 57 insertions(+), 8 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/32/35132/7
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35132?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I056f379715c91ad968f198e112d363a9009dc1c3
Gerrit-Change-Number: 35132
Gerrit-PatchSet: 7
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: arehbein, daniel, laforge, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35079?usp=email )
Change subject: osmo_io: Remove union in struct osmo_io_ops
......................................................................
Patch Set 4:
(1 comment)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/35079/comment/4be00890_58bf8718
PS3, Line 546: if (iofd->io_ops.read_cb)
> i fail to see the problem though, can you explain?
ah ok, read_cb happens to be in the same place as recvfrom_cb, so only checking the one location worked out, but semantics dictate that we should check exactly the member that is matching the current iofd->mode.
Yet this fix seems orthogonal to removing the union?
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35079?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I138d57843edc29000530bb7896bcb239002ecbec
Gerrit-Change-Number: 35079
Gerrit-PatchSet: 4
Gerrit-Owner: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 01 Dec 2023 02:08:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: comment