January 2023 - gerrit-log - lists.osmocom.org

Change in osmocom-bb[master]: layer23: Initial VTY framework to share VTY code between apps

by neels

Attention is currently required from: pespin, msuraev. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30961 ) Change subject: layer23: Initial VTY framework to share VTY code between apps ...................................................................... Patch Set 8: (9 comments) File src/host/layer23/include/osmocom/bb/common/vty.h: https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/508f8382_a8418d4c PS8, Line 6: #include <osmocom/vty/command.h> only vty.h needs to be included for _LAST_OSMOVTY_NODE, right? https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/9b70c6f8_e3809264 PS8, Line 28: (trailing blank line) File src/host/layer23/include/osmocom/bb/modem/vty.h: https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/fcd4caa1_dc29abf7 PS8, Line 7: #include <osmocom/bb/common/vty.h> none of the includes are necessary for this .h, need only struct vty; here File src/host/layer23/src/common/vty.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/39c6fef8_cadf2d21 PS8, Line 2: * (C) 2010 by Andreas Eversberg <jolly(a)eversberg.eu> old (C) for new file? https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/05231c42_dfdfa71e PS8, Line 172: int l23_vty_go_parent(struct vty *vty) you can completely drop vty_go_parent since libosmocore patch commit d31de237582f6fe3315d61bb9a488d4cda92654e Date: Thu Oct 31 16:09:23 2019 +0100 Subject: vty: track parent nodes also for telnet sessions the only need for a go_parent_cb is if you want to add some very specific action to happen when leaving a node (like restart a socket or something) https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/8ba9eeae_67796002 PS8, Line 186: int l23_vty_init(int (*config_write_ms_node_cb)(struct vty *)) > This may be returning 0 only now but it's an API so I prefer keeping return int and checking result […] funny how in this patch, you don't check the rc of l23_vty_init() calls anywhere. Either check the rc or make it void. File src/host/layer23/src/mobile/vty_interface.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/96c45b4f_76551150 PS8, Line 3209: osmo_talloc_vty_add_cmds(); why is this dropped? possibly better in a separate patch? File src/host/layer23/src/modem/vty.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/3462d512_3649b1e1 PS8, Line 2: * (C) 2010 by Andreas Eversberg <jolly(a)eversberg.eu> (old (C) for new file) https://gerrit.osmocom.org/c/osmocom-bb/+/30961/comment/327bbc15_a933a125 PS8, Line 58: (extra blank line) -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30961 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Iabfb3129199488d790b89884bc1e424f2aca696f Gerrit-Change-Number: 30961 Gerrit-PatchSet: 8 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Sun, 15 Jan 2023 23:54:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmocom-bb[master]: mobile: settings.h: Add missing type forward declaration

by neels

Attention is currently required from: pespin, msuraev. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30976 ) Change subject: mobile: settings.h: Add missing type forward declaration ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30976 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ibdfa905e2bd24f03d23ce114e969647b07a2009f Gerrit-Change-Number: 30976 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: msuraev <msuraev(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Sun, 15 Jan 2023 23:37:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmocom-bb[master]: Move osmocom_ms to a separate file

by neels

Attention is currently required from: pespin, msuraev. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30960 ) Change subject: Move osmocom_ms to a separate file ...................................................................... Patch Set 5: (1 comment) File src/host/layer23/src/common/main.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30960/comment/bcbdfa27_3e26682e PS5, Line 272: llist_add_tail(&ms->entity, &ms_list); > i guess this llist_add would also belong in osmocom_ms_alloc()? i see now you are moving it in a later patch. maybe join the two patches? -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30960 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Icb4891cc1e4a0ecb5f09cb8a84b0ebe1b91a46b8 Gerrit-Change-Number: 30960 Gerrit-PatchSet: 5 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Sun, 15 Jan 2023 23:34:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmocom-bb[master]: layer23: Initialize osmocom_ms further in common code

by neels

Attention is currently required from: pespin, msuraev. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30971 ) Change subject: layer23: Initialize osmocom_ms further in common code ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File src/host/layer23/src/common/ms.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30971/comment/1237b7b7_13cfdb12 PS1, Line 33: ms->name = talloc_strdup(ms, name); > because one thing is what user establishes in VTY and the other is the internal talloc name which ca […] The reason to keep talloc_strdup() here is because this patch is moving previous code. But talloc_strdup() is only needed when ms->name can change, so that we can talloc_free(ms->name) safely upon changing the name. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30971 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ic629cf229167ddd4c533a2abf1b82ad78d1144a9 Gerrit-Change-Number: 30971 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: msuraev <msuraev(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Sun, 15 Jan 2023 23:34:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmocom-bb[master]: layer23: mobile: Several fixes and improvements in show_ms_cmd

by neels

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30969 ) Change subject: layer23: mobile: Several fixes and improvements in show_ms_cmd ...................................................................... Patch Set 1: (1 comment) File src/host/layer23/src/mobile/vty_interface.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30969/comment/b9834d42_9ab678cd PS1, Line 246: "Display specific MS with given name") (is there a common MS_STR that can be used instead? that would be the usual approach) -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30969 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I4a55328e71ec16355974c20275c0e525077252e1 Gerrit-Change-Number: 30969 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Sun, 15 Jan 2023 23:22:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmocom-bb[master]: Move osmocom_ms to a separate file

by neels

Attention is currently required from: pespin, msuraev. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30960 ) Change subject: Move osmocom_ms to a separate file ...................................................................... Patch Set 5: (3 comments) File src/host/layer23/include/osmocom/bb/common/osmocom_data.h: https://gerrit.osmocom.org/c/osmocom-bb/+/30960/comment/ac1d3af1_c2782c01 PS5, Line 3: #include <stdint.h> the above are unrelated changes sneaking into this patch File src/host/layer23/src/common/main.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30960/comment/2d84c209_e1539ff4 PS5, Line 271: ms = osmocom_ms_alloc(l23_ctx); > The osmocom_ms_alloc() might return NULL - you should check for this. (ack, IMO would suffice to OSMO_ASSERT(ms) either here or in osmocom_ms_alloc()) https://gerrit.osmocom.org/c/osmocom-bb/+/30960/comment/a749b888_29b18707 PS5, Line 272: llist_add_tail(&ms->entity, &ms_list); i guess this llist_add would also belong in osmocom_ms_alloc()? -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30960 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Icb4891cc1e4a0ecb5f09cb8a84b0ebe1b91a46b8 Gerrit-Change-Number: 30960 Gerrit-PatchSet: 5 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Sun, 15 Jan 2023 23:19:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmocom-bb[master]: layer23: Add missing header dependencies to several files

by neels

Attention is currently required from: pespin, msuraev. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30959 ) Change subject: layer23: Add missing header dependencies to several files ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: the aim for .h files is to minimize the amount of other headers included. Instead, .c files should include .h files. In a .h file, for example, opaque struct definitions are more desirable than an include of another .h with the full struct definition. The reason is that when .h files include other .h files, a circular include dependency becomes more likely. It can "cascade" unnecessary dependencies across APIs. When we aim each .h file to remain as opaque as possible, we avoid circular dependencies. (also, includes between .h files can cause more .o files to be recompiled than necessary for small changes to .h files) So in this patch, i would re-argue every .h file where an #include is added -- it compiled before, why add #includes? -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30959 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I9819b12d1c24f6ee197daa887452b09418d689e8 Gerrit-Change-Number: 30959 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Sun, 15 Jan 2023 23:11:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmocom-bb[master]: cosmetic: layer23: Fix trailing whitespace

by neels

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30958 ) Change subject: cosmetic: layer23: Fix trailing whitespace ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30958 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ibc464923ac8add74b2b6d57d183fb307be650679 Gerrit-Change-Number: 30958 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Sun, 15 Jan 2023 23:03:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmocom-bb[master]: layer23: misc/log.h: Fix missing pragma once

by neels

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30957 ) Change subject: layer23: misc/log.h: Fix missing pragma once ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30957 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ie57357224ba5ed2bf691e2912ede7ee19012b6dd Gerrit-Change-Number: 30957 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Sun, 15 Jan 2023 23:01:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

Change in osmocom-bb[master]: layer23: common/sim.h: Fix missing pragma once

by neels

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30956 ) Change subject: layer23: common/sim.h: Fix missing pragma once ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30956 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Icebe803c7896347915ed72b80d5504d15d717ef3 Gerrit-Change-Number: 30956 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Sun, 15 Jan 2023 23:01:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 5 months

1
0
0 0

2025

2024

2023

2022

gerrit-log January 2023