Attention is currently required from: laforge, neels, pespin.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35212?usp=email )
Change subject: ecu: fix alignment of fr_ecu_state
......................................................................
Patch Set 7:
(4 comments)
Commit Message:
https://gerrit.osmocom.org/c/libosmocore/+/35212/comment/3a391b30_23de5c8b
PS5, Line 7: ecu: force alignment of member data in struct osmo_ecu_state
> you probably need to update the whole commit description since the approach changed.
I have now fixed it (hopefully). The description of the is-situation should still apply.
File src/codec/ecu_fr.c:
https://gerrit.osmocom.org/c/libosmocore/+/35212/comment/028d33bb_1cf98811
PS4, Line 92: struct osmo_ecu_state ecu_state;
> Add a comment that ecu_state should be the first in the struct. […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/35212/comment/3f5335ac_fffc3a17
PS4, Line 294: return (struct osmo_ecu_state*) fr;
> Ah sorry, not in this precise line. But here it's simply a matter of doing: […]
Done
File src/codec/ecu_fr.c:
https://gerrit.osmocom.org/c/libosmocore/+/35212/comment/781858f8_fd64fa1e
PS5, Line 315: struct fr_ecu_state *fr = (struct fr_ecu_state *) st;
> It removes the need of having one struct being the first field of the other one, plus avoid having p […]
Done
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35212?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: I28672856e8e8f47e04ffe09ee3e07b577108cdc7
Gerrit-Change-Number: 35212
Gerrit-PatchSet: 7
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 04 Jan 2024 12:56:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge, neels, pespin.
Hello Jenkins Builder, laforge, neels, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/35212?usp=email
to look at the new patch set (#7).
The following approvals got outdated and were removed:
Code-Review-1 by pespin, Verified-1 by Jenkins Builder
Change subject: ecu: fix alignment of fr_ecu_state
......................................................................
ecu: fix alignment of fr_ecu_state
The member data[0] in struct osmo_ecu_state is used as an anchor to
attach private structs for a concrete ECU implementation. This works by
allocating more memory then struct osmo_ecu_state actually needs and
then using the excess memory to store the private struct of the concrete
ECU implementation.
However, this poses a problem since data[0] is at the end of the struct
it may land in an unaligned position. This also means that the struct we
store there is also unaligned.
We should fix this enclosing the public struct osmo_ecu_state into our
private struct fr_ecu_state. Then we can use container_of to cast from
osmo_ecu_state to fr_ecu_state and correct alignment is ensured as well.
Related: OS#6286
Change-Id: I28672856e8e8f47e04ffe09ee3e07b577108cdc7
---
M src/codec/ecu_fr.c
1 file changed, 37 insertions(+), 13 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/12/35212/7
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35212?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: I28672856e8e8f47e04ffe09ee3e07b577108cdc7
Gerrit-Change-Number: 35212
Gerrit-PatchSet: 7
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: matanp.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35483?usp=email )
Change subject: ctrl: Add radio link timeout
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35483?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ic9532d4b051d34e71c91aaff545fb3dfa7d7c8b2
Gerrit-Change-Number: 35483
Gerrit-PatchSet: 1
Gerrit-Owner: matanp <matan1008(a)gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: matanp <matan1008(a)gmail.com>
Gerrit-Comment-Date: Thu, 04 Jan 2024 12:36:08 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment