September 2022 - gerrit-log - lists.osmocom.org

Change in osmo-msc[master]: SMPP: move read/write callbacks to libsmpputil

by neels

Attention is currently required from: osmith, msuraev. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28848 ) Change subject: SMPP: move read/write callbacks to libsmpputil ...................................................................... Patch Set 28: (6 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-msc/+/28848/comment/0f4c540b_64b8ea6a PS28, Line 7: SMPP: move read/write callbacks to libsmpputil would be good to give a reason for the move / which other caller is currently expected File include/osmocom/smpp/smpp.h: https://gerrit.osmocom.org/c/osmo-msc/+/28848/comment/dcbf9993_b50aaa3a PS28, Line 51: + * osmo_fd to properly check for errors. you have diff '+' leaked into the C code https://gerrit.osmocom.org/c/osmo-msc/+/28848/comment/683ef905_fe48acca PS28, Line 53: + * an error. The code there should handle closing the connection. I would prefer to keep this macro confined locally to a .c file, i guess smpp_utils.c - contains flow control (not local to a .c file but in a header file) - requiring a label - requiring a specific return type in calling code If this makes it into public API, it needs a more detailed explanation, particularly about the 'return 0' meaning "try again". I guess best would be a code example of what the macro expects. https://gerrit.osmocom.org/c/osmo-msc/+/28848/comment/95e06b96_0280d326 PS28, Line 55: + * projects as well. hmm idk. this should have a name that is different from the general libosmocore-style to avoid naming conflicts. For example if you plan to move it to libosmocore, then you can clearly switch from one to the other. like SMPP_CHECK_FD_READ File src/libsmpputil/smpp_utils.c: https://gerrit.osmocom.org/c/osmo-msc/+/28848/comment/75c3dae7_8579125f PS28, Line 58: /* !\brief call-back when per-ESME TCP socket has some data to be read */ drop \brief, see https://osmocom.org/projects/cellular-infrastructure/wiki/Guidelines_for_AP… https://gerrit.osmocom.org/c/osmo-msc/+/28848/comment/8448af0d_bece25b5 PS28, Line 117: int esme_write_callback(struct esme *esme, int fd, struct msgb *msg) documentation for args and rc? (I tried to understand the 'exit(99)' below and found no explanation for rc == 0) -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28848 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I875eb5249004d3a960aee46c5099592d18fcaa76 Gerrit-Change-Number: 28848 Gerrit-PatchSet: 28 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Thu, 15 Sep 2022 11:43:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 9 months

1
0
0 0

Change in docker-playground[master]: add ttcn3-hnbgw-test variant with-pfcp

by pespin

Attention is currently required from: osmith, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/docker-playground/+/29339 ) Change subject: add ttcn3-hnbgw-test variant with-pfcp ...................................................................... Patch Set 3: (1 comment) File ttcn3-hnbgw-test/jenkins.sh: https://gerrit.osmocom.org/c/docker-playground/+/29339/comment/6968df9e_198… PS3, Line 82: echo Starting container with STP > is it worth the effort? this is just a ci shell script, the single user is our jenkins, it works in […] If you want to understand what's run here, and how run a subset of the things here: yes it helps a lot. Also if for instance I want to temporarily run some program with gdb, then I need to change it in one place, not find out where. So sparing you now one round of running the script may save others time and efforts later. -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/29339 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: I02b60941343000a4618e95f56326bec170c32bfe Gerrit-Change-Number: 29339 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Thu, 15 Sep 2022 11:31:22 +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

2 years, 9 months

1
0
0 0

Change in osmo-ci[master]: lint: ignore MACRO_WITH_FLOW_CONTROL

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ci/+/29359 ) Change subject: lint: ignore MACRO_WITH_FLOW_CONTROL ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ci/+/29359 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ci Gerrit-Branch: master Gerrit-Change-Id: I79da5a426db59031e3b16aecedeaa1498c91e847 Gerrit-Change-Number: 29359 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Thu, 15 Sep 2022 11:29:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 9 months

1
0
0 0

Change in docker-playground[master]: add ttcn3-hnbgw-test variant with-pfcp

by neels

Attention is currently required from: osmith, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/docker-playground/+/29339 ) Change subject: add ttcn3-hnbgw-test variant with-pfcp ...................................................................... Patch Set 3: (1 comment) File ttcn3-hnbgw-test/jenkins.sh: https://gerrit.osmocom.org/c/docker-playground/+/29339/comment/50cc92fc_d37… PS3, Line 82: echo Starting container with STP > It would be really great if you moved duplicated chunks of code into functions, like start_stp, star […] is it worth the effort? this is just a ci shell script, the single user is our jenkins, it works in this state and changing it would mean another round of testing. The result is more indirection/complexity in calling some shell commands. unlikely that this is a substantial help in future maintenance. -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/29339 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: I02b60941343000a4618e95f56326bec170c32bfe Gerrit-Change-Number: 29339 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 15 Sep 2022 11:08:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 9 months

1
0
0 0

Change in osmo-msc[master]: SMPP: move read/write callbacks to libsmpputil

by osmith

Attention is currently required from: osmith. osmith has removed a vote from this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28848 ) Change subject: SMPP: move read/write callbacks to libsmpputil ...................................................................... Removed Verified-1 by Jenkins Builder (1000002) -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28848 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I875eb5249004d3a960aee46c5099592d18fcaa76 Gerrit-Change-Number: 28848 Gerrit-PatchSet: 28 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: deleteVote

2 years, 9 months

1
0
0 0

Change in osmo-msc[master]: SMPP: move read/write callbacks to libsmpputil

by msuraev

Attention is currently required from: osmith. msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28848 ) Change subject: SMPP: move read/write callbacks to libsmpputil ...................................................................... Patch Set 28: (1 comment) Patchset: PS28: > I also dislike flow control macro but in this case it doesn't make sense indeed. Seems like I can't remove jenkins vote: the trash icon is only visible next to my own vote. -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28848 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I875eb5249004d3a960aee46c5099592d18fcaa76 Gerrit-Change-Number: 28848 Gerrit-PatchSet: 28 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Thu, 15 Sep 2022 09:43:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 9 months

1
0
0 0

Change in osmo-msc[master]: SMPP: move read/write callbacks to libsmpputil

by msuraev

Attention is currently required from: osmith. msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28848 ) Change subject: SMPP: move read/write callbacks to libsmpputil ...................................................................... Patch Set 28: Verified+1 (1 comment) Patchset: PS28: > FYI you can just override the jenkins linter error if it doesn't make sense (remove the -1 build ver […] I also dislike flow control macro but in this case it doesn't make sense indeed. -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28848 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I875eb5249004d3a960aee46c5099592d18fcaa76 Gerrit-Change-Number: 28848 Gerrit-PatchSet: 28 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Thu, 15 Sep 2022 09:42:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 9 months

1
0
0 0

Change in osmo-msc[master]: SMPP: move read/write callbacks to libsmpputil

by osmith

Attention is currently required from: msuraev. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28848 ) Change subject: SMPP: move read/write callbacks to libsmpputil ...................................................................... Patch Set 28: (1 comment) Patchset: PS28: FYI you can just override the jenkins linter error if it doesn't make sense (remove the -1 build verification vote from jenkins and add your own +1 build verification). We should probably disable the check for MACRO_WITH_FLOW_CONTROL? -> https://gerrit.osmocom.org/c/osmo-ci/+/29359 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28848 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I875eb5249004d3a960aee46c5099592d18fcaa76 Gerrit-Change-Number: 28848 Gerrit-PatchSet: 28 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Thu, 15 Sep 2022 09:00:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 9 months

1
0
0 0

Change in osmo-ci[master]: lint: ignore MACRO_WITH_FLOW_CONTROL

by osmith

osmith has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ci/+/29359 ) Change subject: lint: ignore MACRO_WITH_FLOW_CONTROL ...................................................................... lint: ignore MACRO_WITH_FLOW_CONTROL It seems that we don't care about this one, e.g. here: https://gerrit.osmocom.org/c/osmo-msc/+/28848 Change-Id: I79da5a426db59031e3b16aecedeaa1498c91e847 --- M lint/checkpatch/checkpatch_osmo.sh 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ci refs/changes/59/29359/1 diff --git a/lint/checkpatch/checkpatch_osmo.sh b/lint/checkpatch/checkpatch_osmo.sh index b0c3985..7a60ad6 100755 --- a/lint/checkpatch/checkpatch_osmo.sh +++ b/lint/checkpatch/checkpatch_osmo.sh @@ -68,6 +68,7 @@ # * LINE_CONTINUATIONS: false positives # * LINE_SPACING: we don't always put a blank line after declarations # * LONG_LINE*: should be 120 chars, but exceptions are done often so don't fail here +# * MACRO_WITH_FLOW_CONTROL: not followed # * MISSING_SPACE: warns about breaking strings at space characters, not useful for long strings of hex chars # * PREFER_DEFINED_ATTRIBUTE_MACRO: macros like __packed not defined in libosmocore # * PREFER_FALLTHROUGH: pseudo keyword macro "fallthrough" is not defined in libosmocore @@ -108,6 +109,7 @@ --ignore LONG_LINE \ --ignore LONG_LINE_COMMENT \ --ignore LONG_LINE_STRING \ + --ignore MACRO_WITH_FLOW_CONTROL \ --ignore MISSING_SPACE \ --ignore PREFER_DEFINED_ATTRIBUTE_MACRO \ --ignore PREFER_FALLTHROUGH \ -- To view, visit https://gerrit.osmocom.org/c/osmo-ci/+/29359 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ci Gerrit-Branch: master Gerrit-Change-Id: I79da5a426db59031e3b16aecedeaa1498c91e847 Gerrit-Change-Number: 29359 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 9 months

1
0
0 0

Change in osmo-msc[master]: SMPP: move read/write callbacks to libsmpputil

by Jenkins Builder

Attention is currently required from: msuraev. Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28848 ) Change subject: SMPP: move read/write callbacks to libsmpputil ...................................................................... Patch Set 28: (1 comment) File include/osmocom/smpp/smpp.h: Robot Comment from checkpatch (run ID jenkins-gerrit-osmo-msc-lint-361): https://gerrit.osmocom.org/c/osmo-msc/+/28848/comment/7ef84a92_d318371e PS28, Line 57: #define OSMO_FD_CHECK_READ(rc, err_label) do { \ Macros with flow control statements should be avoided -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28848 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I875eb5249004d3a960aee46c5099592d18fcaa76 Gerrit-Change-Number: 28848 Gerrit-PatchSet: 28 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Thu, 15 Sep 2022 03:29:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 9 months

1
0
0 0

2025

2024

2023

2022

gerrit-log September 2022