Attention is currently required from: osmith, fixeria.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-cbc/+/28602 )
Change subject: Add initial SBc-AP support to osmo-cbc
......................................................................
Patch Set 11:
(1 comment)
File include/osmocom/cbc/internal.h:
https://gerrit.osmocom.org/c/osmo-cbc/+/28602/comment/bb50a98d_37ea249c
PS10, Line 30: enum sbcap_server_event {
> If the meaning is not the same, why names and comments are? Just wondering.
Because this needs to be further improved. But this is working as it is, don't ask me to impelemnt 100% of the protocol and change 10000 more lines in one commit.
The commit states it clearly: this is a first step adding initial implementation.
--
To view, visit https://gerrit.osmocom.org/c/osmo-cbc/+/28602
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-cbc
Gerrit-Branch: master
Gerrit-Change-Id: Ib278bc1d1a74459814016fef7a8fe21cc29d46c9
Gerrit-Change-Number: 28602
Gerrit-PatchSet: 11
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 18 Jul 2022 16:22:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: fixeria.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-cbc/+/28646 )
Change subject: vty: Fix call to OSMO_STRBUF_PRINTF
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Why not just squash this fixup into 28602?
Because it has its own history, it was only spotted under really specific conditions, so it's nice having history about why this is done this way, to avoid somebody turning it back or asking me in review "why didn't you do it that other way" or "why did you change that now?".
--
To view, visit https://gerrit.osmocom.org/c/osmo-cbc/+/28646
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-cbc
Gerrit-Branch: master
Gerrit-Change-Id: I90e7bdbe9f01dae9269ae4850bc2d7391fee71ec
Gerrit-Change-Number: 28646
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 18 Jul 2022 16:19:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/28647 )
Change subject: stream: getsockopt ret socklen_t is unsigned
......................................................................
stream: getsockopt ret socklen_t is unsigned
Found out by following gcc warning:
"""
libosmo-netif/src/stream.c:147:11: warning: argument to variable-length array may be too large due to conversion from ‘int’ to ‘unsigned int’ [-Wvla-larger-than=]
147 | uint8_t buf[sctp_sockopt_event_subscribe_size];
| ^~~
"""
Change-Id: I181ae5c0480788fc96abd2bb1edf003244884c8f
---
M src/stream.c
1 file changed, 3 insertions(+), 3 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/47/28647/1
diff --git a/src/stream.c b/src/stream.c
index e6b731c..cdd0eae 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -87,7 +87,7 @@
return -1;
}
-static int sctp_sockopt_event_subscribe_size = 0;
+static unsigned int sctp_sockopt_event_subscribe_size = 0;
static int determine_sctp_sockopt_event_subscribe_size(void)
{
@@ -96,7 +96,7 @@
int sd, rc;
/* only do this once */
- if (sctp_sockopt_event_subscribe_size != 0)
+ if (sctp_sockopt_event_subscribe_size > 0)
return 0;
sd = socket(AF_INET, SOCK_STREAM, IPPROTO_SCTP);
@@ -108,7 +108,7 @@
if (rc < 0)
return rc;
- sctp_sockopt_event_subscribe_size = buf_len;
+ sctp_sockopt_event_subscribe_size = (unsigned int)buf_len;
LOGP(DLINP, LOGL_INFO, "sizes of 'struct sctp_event_subscribe': compile-time %zu, kernel: %u\n",
sizeof(struct sctp_event_subscribe), sctp_sockopt_event_subscribe_size);
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/28647
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I181ae5c0480788fc96abd2bb1edf003244884c8f
Gerrit-Change-Number: 28647
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-cbc/+/28646 )
Change subject: vty: Fix call to OSMO_STRBUF_PRINTF
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Why not just squash this fixup into 28602?
--
To view, visit https://gerrit.osmocom.org/c/osmo-cbc/+/28646
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-cbc
Gerrit-Branch: master
Gerrit-Change-Id: I90e7bdbe9f01dae9269ae4850bc2d7391fee71ec
Gerrit-Change-Number: 28646
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 18 Jul 2022 16:03:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: osmith, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-cbc/+/28602 )
Change subject: Add initial SBc-AP support to osmo-cbc
......................................................................
Patch Set 11:
(3 comments)
File include/osmocom/cbc/cbc_data.h:
https://gerrit.osmocom.org/c/osmo-cbc/+/28602/comment/306ccc6a_175b3cd4
PS10, Line 23: CBC_PEER_PROTO_SBcAP
> It's really arguable whether it makes sense to add a comma at the end of the last one.
Well, there was one before this patch. And it's generally a good practice making patches adding new entries easier to read. In any case, I am not going to block because of that.
File include/osmocom/cbc/internal.h:
https://gerrit.osmocom.org/c/osmo-cbc/+/28602/comment/160d2fad_ea0249b6
PS10, Line 30: enum sbcap_server_event {
> No, because it's not (or will not) be exactly the same set, nor exactly the same meaning.
If the meaning is not the same, why names and comments are? Just wondering.
File src/sbcap_server_fsm.c:
https://gerrit.osmocom.org/c/osmo-cbc/+/28602/comment/9df0ccdf_c4ee5b6e
PS8, Line 303: //else
> commented out code, remove?
Ack. Neither it's marked as FIXME nor as TODO.
It's better to use #if 0 ... #else ... #endif here if it should be kept.
--
To view, visit https://gerrit.osmocom.org/c/osmo-cbc/+/28602
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-cbc
Gerrit-Branch: master
Gerrit-Change-Id: Ib278bc1d1a74459814016fef7a8fe21cc29d46c9
Gerrit-Change-Number: 28602
Gerrit-PatchSet: 11
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 18 Jul 2022 15:57:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
