pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/21243 )
Change subject: ns2: add support for frame relay ...................................................................... Patch Set 2: (5 comments) https://gerrit.osmocom.org/c/libosmocore/+/21243/1/src/gb/frame_relay.c File src/gb/frame_relay.c: https://gerrit.osmocom.org/c/libosmocore/+/21243/1/src/gb/frame_relay.c@106 PS1, Line 106: /* RX Message: 14 [ 00 01 03 08 00 75 95 01 01 00 03 02 01 00 ] */ > what's bad about some examples of the messages in a comment? well I'd expect then at least some comment explaining what these list of bytes mean, or to which kind of message / layer they belong. https://gerrit.osmocom.org/c/libosmocore/+/21243/1/src/gb/frame_relay.c@343 PS1, Line 343: /* TODO: implement FRNET free */ > it's complicated ;D. See the new big comment. I don't see a new big comment in here in new patch version. https://gerrit.osmocom.org/c/libosmocore/+/21243/2/src/gb/frame_relay.c File src/gb/frame_relay.c: https://gerrit.osmocom.org/c/libosmocore/+/21243/2/src/gb/frame_relay.c@345 PS2, Line 345: LOGPFRL(link, LOGL_ERROR, "STATUS-ENQ are not support for role user\n"); you missed the "support"->"supported" part. https://gerrit.osmocom.org/c/libosmocore/+/21243/2/src/gb/frame_relay.c@948 PS2, Line 948: /* TODO: rework osmo_fr_dlc_alloc/free with handling it's own memory. This is the big new comment? https://gerrit.osmocom.org/c/libosmocore/+/21243/2/src/gb/frame_relay.c@953 PS2, Line 953: * by the frame relay because the network is configuring the dlc. I guess this is where you want to use the API include/osmocom/core/use_count.h -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/21243 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id3b49f93d33c271f77cd9c9db03cde6b727a4d30 Gerrit-Change-Number: 21243 Gerrit-PatchSet: 2 Gerrit-Owner: lynxis lazus <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <[email protected]> Gerrit-CC: laforge <[email protected]> Gerrit-Comment-Date: Tue, 24 Nov 2020 10:37:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <[email protected]> Comment-In-Reply-To: pespin <[email protected]> Comment-In-Reply-To: lynxis lazus <[email protected]> Gerrit-MessageType: comment
