matanp has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35476?usp=email )
Change subject: ctrl: Add rach expiry timeout
......................................................................
Set Ready For Review
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35476?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: I74a402a3800d245e4bc3052ccb53d8d2660e9f52
Gerrit-Change-Number: 35476
Gerrit-PatchSet: 1
Gerrit-Owner: matanp <matan1008(a)gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Comment-Date: Tue, 02 Jan 2024 11:57:54 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: dexter, laforge, neels.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35212?usp=email )
Change subject: ecu: force alignment of member data in struct osmo_ecu_state
......................................................................
Patch Set 6:
(1 comment)
File src/codec/ecu_fr.c:
https://gerrit.osmocom.org/c/libosmocore/+/35212/comment/81ccd77f_c408102a
PS5, Line 315: struct fr_ecu_state *fr = (struct fr_ecu_state *) st;
> Thanks. I have added this now as suggested, but I still do not understand where the advantage is. […]
It removes the need of having one struct being the first field of the other one, plus avoid having problems in the future if somebody decides to add other fields.
This way the compiler is responsible for calculating the address instead of tricking it through explicit casting.
--
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: 6
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: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 02 Jan 2024 11:30:55 +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.
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35212?usp=email )
Change subject: ecu: force alignment of member data in struct osmo_ecu_state
......................................................................
Patch Set 6:
(1 comment)
File src/codec/ecu_fr.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-13397):
https://gerrit.osmocom.org/c/libosmocore/+/35212/comment/5bf2c239_a3f747a3
PS6, Line 300: return (struct fr_ecu_state *)container_of(st, struct fr_ecu_state, ecu_state);
please, no spaces at the start of a line
--
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: 6
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: Tue, 02 Jan 2024 11:27:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
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: force alignment of member data in struct osmo_ecu_state
......................................................................
Patch Set 6:
(1 comment)
File src/codec/ecu_fr.c:
https://gerrit.osmocom.org/c/libosmocore/+/35212/comment/204ef9d4_51209530
PS5, Line 315: struct fr_ecu_state *fr = (struct fr_ecu_state *) st;
> Here and in the function below is where you need container_of: […]
Thanks. I have added this now as suggested, but I still do not understand where the advantage is. container_of gets st, which is of type osmo_ecu_state but when calling container_of we specify it as struct fr_ecu_state. And we request the container address of the member ecu_state, which is struct fr_ecu_state. I see that this works but I do not see any practical advantage. We still need to have the ecu_state member in struct fr_ecu_state.
--
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: 6
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: Tue, 02 Jan 2024 11:27:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(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 (#6).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: ecu: force alignment of member data in struct osmo_ecu_state
......................................................................
ecu: force alignment of member data in struct osmo_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
simply 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, 36 insertions(+), 13 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/12/35212/6
--
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: 6
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