Attention is currently required from: laforge, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/32320 )
Change subject: sccp_scoc.c: fix infinite loop on conn ID exhaustion
......................................................................
Patch Set 4:
(1 comment)
File src/sccp_scoc.c:
https://gerrit.osmocom.org/c/libosmo-sccp/+/32320/comment/5b4772aa_839f053a
PS3, Line 562: while ((max_attempts--) > 0) {
> i'm only starting to get a handle on the LIKELY and UNLIKELY stuff... […]
Done
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/32320
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: Ib64e0cb1ae0cc8b7bebcb2a352d4068b496b304a
Gerrit-Change-Number: 32320
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 10 May 2023 00:18:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge, pespin.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-sccp/+/32320
to look at the new patch set (#4).
Change subject: sccp_scoc.c: fix infinite loop on conn ID exhaustion
......................................................................
sccp_scoc.c: fix infinite loop on conn ID exhaustion
Looking for an unused SCCP connection ID has no exit condition if all
connection IDs are in use. However unlikely it is that there are
16777215 active connections on one SCCP instance, add an exit condition.
Do so by implementing the ID lookup in a new separate function, which
qualifies for public API (upcoming patch).
So far, use an exit condition closest to previous behavior: iterate the
entire SCCP conn id number space before exiting in failure.
Change-Id: Ib64e0cb1ae0cc8b7bebcb2a352d4068b496b304a
---
M src/sccp_scoc.c
1 file changed, 41 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/20/32320/4
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/32320
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: Ib64e0cb1ae0cc8b7bebcb2a352d4068b496b304a
Gerrit-Change-Number: 32320
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(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: laforge, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/32320 )
Change subject: sccp_scoc.c: fix infinite loop on conn ID exhaustion
......................................................................
Patch Set 3:
(2 comments)
File src/sccp_scoc.c:
https://gerrit.osmocom.org/c/libosmo-sccp/+/32320/comment/0cebe39f_ca2b74bb
PS3, Line 549: int max_attempts = 0x00FFFFFE;
> this would better be an unsigned, but fine anyway.
there is a 'while (max_attempts > 0)' below, so I'd rather use an unsigned value -- it's not really needed here, sure, but when there's subtraction and comparison, it's just generally saner to use a signed type. If a bug would accidentally 'max_attempts--' too often, an unsigned type could make that an infinite loop. A signed type would still exit.
https://gerrit.osmocom.org/c/libosmo-sccp/+/32320/comment/6606ee89_423886a1
PS3, Line 562: while ((max_attempts--) > 0) {
> OSMO_LIKELY(), but not critical.
i'm only starting to get a handle on the LIKELY and UNLIKELY stuff... so you mean like this? <points at next patch set>
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/32320
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: Ib64e0cb1ae0cc8b7bebcb2a352d4068b496b304a
Gerrit-Change-Number: 32320
Gerrit-PatchSet: 3
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 10 May 2023 00:15:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
laforge has submitted this change. ( https://gerrit.osmocom.org/c/pysim/+/32406 )
Change subject: ts_51_011: fix EF_ServiceTable: use self for static method
......................................................................
ts_51_011: fix EF_ServiceTable: use self for static method
Even though _bit_byte_offset_for_service() is a @staticmethod, it's
still available via self, just like any non-static method.
Change-Id: I3590dda341d534deb1b7f4743ea31ab16dbd6912
---
M pySim/ts_51_011.py
1 file changed, 14 insertions(+), 4 deletions(-)
Approvals:
laforge: Looks good to me, approved
Jenkins Builder: Verified
Objections:
pespin: I would prefer this is not merged as is
diff --git a/pySim/ts_51_011.py b/pySim/ts_51_011.py
index 3dc21c8..222bdaf 100644
--- a/pySim/ts_51_011.py
+++ b/pySim/ts_51_011.py
@@ -670,16 +670,14 @@
bin_len = 0
for srv in in_json.keys():
service_nr = int(srv)
- (byte_offset, bit_offset) = EF_ServiceTable._bit_byte_offset_for_service(
- service_nr)
+ (byte_offset, bit_offset) = self._bit_byte_offset_for_service(service_nr)
if byte_offset >= bin_len:
bin_len = byte_offset+1
# encode the actual data
out = bytearray(b'\x00' * bin_len)
for srv in in_json.keys():
service_nr = int(srv)
- (byte_offset, bit_offset) = EF_ServiceTable._bit_byte_offset_for_service(
- service_nr)
+ (byte_offset, bit_offset) = self._bit_byte_offset_for_service(service_nr)
bits = 0
if in_json[srv]['allocated'] == True:
bits |= 1
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/32406
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I3590dda341d534deb1b7f4743ea31ab16dbd6912
Gerrit-Change-Number: 32406
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: pespin, fixeria.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/32406 )
Change subject: ts_51_011: fix EF_ServiceTable: use self for static method
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Patchset:
PS1:
> inheritance in a static method? I'm not an OOP expert but that doesn't make much sense imho, since t […]
I'm not an OOP expert either, but I see nothing conceptually wrong with a derived class overloading a static method in python. The only rason it's static is that we don't need 'self' in it, as it purely operates on data outside of the instance. And a derived class may well override that.
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/32406
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I3590dda341d534deb1b7f4743ea31ab16dbd6912
Gerrit-Change-Number: 32406
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 10 May 2023 00:14:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment