Change in libosmocore[master]: WIP: gprs_ns2: add new vty2

[lynxis lazus]

lynxis lazus has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/21487 )

Change subject: WIP: gprs_ns2: add new vty2
......................................................................


Patch Set 18:

(9 comments)

https://gerrit.osmocom.org/c/libosmocore/+/21487/18/include/osmocom/gprs/gprs_ns2.h 
File include/osmocom/gprs/gprs_ns2.h:

https://gerrit.osmocom.org/c/libosmocore/+/21487/18/include/osmocom/gprs/gprs_ns2.h@45 
PS18, Line 45: 	GPRS_NS2_LL_UNDEF,	/*!< undefined used by vty */
> what do you mean here? "undefined use by vty"? or "undefined is used by bty"? If the second, simply  […]
this sentence makes me smile. Even it wasn't meant that way.


https://gerrit.osmocom.org/c/libosmocore/+/21487/18/src/gb/gprs_ns2_internal.h 
File src/gb/gprs_ns2_internal.h:

https://gerrit.osmocom.org/c/libosmocore/+/21487/18/src/gb/gprs_ns2_internal.h@188 
PS18, Line 188: 
> beautiful new spacing not related to the commit.
Ack


https://gerrit.osmocom.org/c/libosmocore/+/21487/18/src/gb/gprs_ns2_sns.c 
File src/gb/gprs_ns2_sns.c:

https://gerrit.osmocom.org/c/libosmocore/+/21487/18/src/gb/gprs_ns2_sns.c@1541 
PS18, Line 1541:  *  \param[in] stats Whether or not statistics shall also be printed */
> ninja param, not showing up in the function!
it's not often that the documentation precedes


https://gerrit.osmocom.org/c/libosmocore/+/21487/18/src/gb/gprs_ns2_sns.c@1554 
PS18, Line 1554: 		vty_out(vty, "  ip-sns %s %d%s", addr_str.ip, addr_str.port, VTY_NEWLINE);
> port is probably %u
Ack


https://gerrit.osmocom.org/c/libosmocore/+/21487/4/src/gb/gprs_ns2_vty2.c 
File src/gb/gprs_ns2_vty2.c:

https://gerrit.osmocom.org/c/libosmocore/+/21487/4/src/gb/gprs_ns2_vty2.c@670 
PS4, Line 670: "A unique bind identifier created by ns bind\n"
             :       "Remote IPv4 Address\n" "Remote IPv6 Address\n"
             :       "Remote UDP Port\n"
> could also all be #defines as we need them multiple times, even in the same order
Done


https://gerrit.osmocom.org/c/libosmocore/+/21487/15/src/gb/gprs_ns2_vty2.c 
File src/gb/gprs_ns2_vty2.c:

https://gerrit.osmocom.org/c/libosmocore/+/21487/15/src/gb/gprs_ns2_vty2.c@232 
PS15, Line 232: /* TODO: create a function GPRS_NS2_LL_UDP -> udp function */
> don't we already have a value_string for that?  we have get_string_value(), not just get_value_strin […]
Done


https://gerrit.osmocom.org/c/libosmocore/+/21487/18/src/gb/gprs_ns2_vty2.c 
File src/gb/gprs_ns2_vty2.c:

https://gerrit.osmocom.org/c/libosmocore/+/21487/18/src/gb/gprs_ns2_vty2.c@220 
PS18, Line 220: /* TODO: add fr/gre */
> what about this todo.
fr/gre is a todo of the ns2 implementation. IMHO: without test equipment this isn't going to be implemented.


https://gerrit.osmocom.org/c/libosmocore/+/21487/18/src/gb/gprs_ns2_vty2.c@362 
PS18, Line 362: 			vty_out(vty, "nsvc udp %s %s %d%s",
> ports as %u?
Done


https://gerrit.osmocom.org/c/libosmocore/+/21487/18/src/gb/gprs_ns2_vty2.c@408 
PS18, Line 408: 	// TODO timer
> this is missing?
Done



-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/21487
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I163279cf57e84198dc8c53e1c109f5a9474670e9
Gerrit-Change-Number: 21487
Gerrit-PatchSet: 18
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-CC: daniel <dwillmann at sysmocom.de>
Gerrit-Comment-Date: Tue, 22 Dec 2020 12:24:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge at osmocom.org>
Comment-In-Reply-To: pespin <pespin at sysmocom.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20201222/cbd5264f/attachment.htm>