Change in libosmocore[master]: Gb: add a second NS implementation

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

pespin gerrit-no-reply at lists.osmocom.org
Wed Sep 9 11:30:51 UTC 2020


pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/19417 )

Change subject: Gb: add a second NS implementation
......................................................................


Patch Set 24:

(4 comments)

https://gerrit.osmocom.org/c/libosmocore/+/19417/24/include/osmocom/gprs/gprs_ns2.h 
File include/osmocom/gprs/gprs_ns2.h:

https://gerrit.osmocom.org/c/libosmocore/+/19417/24/include/osmocom/gprs/gprs_ns2.h@85 
PS24, Line 85: 			/* TODO: implement resource distribution */
> I'll out comment the link selector parameter. […]
If you declare a variable of type "struct osmo_gprs_ns2_prim" in a program an then change the size of the struct in the lib, for sure you are breaking the ABI (well in the case of a union is really depends on whether it's already the option with the biggest size or not, but you get the point).

If you add stuff where the TODO is, it's not the latest field in the struct, so again you are breaking the ABI.


https://gerrit.osmocom.org/c/libosmocore/+/19417/24/src/gb/gprs_ns2.c 
File src/gb/gprs_ns2.c:

https://gerrit.osmocom.org/c/libosmocore/+/19417/24/src/gb/gprs_ns2.c@882 
PS24, Line 882: static bool gprs_fsm_vc_registered = false;
> I don't care. Just took this over from NS1. […]
I think we usually use constructors to register GFSM and all that. No strong opinion though.


https://gerrit.osmocom.org/c/libosmocore/+/19417/24/src/gb/gprs_ns2_frgre.c 
File src/gb/gprs_ns2_frgre.c:

https://gerrit.osmocom.org/c/libosmocore/+/19417/24/src/gb/gprs_ns2_frgre.c@524 
PS24, Line 524: 		rc = handle_nsfrgre_read(bfd);
> Did I missed the free?
I didn't check, just pointing it out in case you didn't check or forgot about the possibility :)


https://gerrit.osmocom.org/c/libosmocore/+/19417/24/src/gb/gprs_ns2_message.c 
File src/gb/gprs_ns2_message.c:

https://gerrit.osmocom.org/c/libosmocore/+/19417/24/src/gb/gprs_ns2_message.c@64 
PS24, Line 64: int gprs_ns2_validate_reset(struct gprs_ns2_vc *nsvc, struct msgb *msg, struct tlv_parsed *tp, uint8_t *cause)
> what do you mean?
If 0 means success and 1 error, usually we return negative values. Moreover, otherwise why is an int returned and not an unsigned int?



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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I3525beef205588dfab9d3880a34115f1a2676e48
Gerrit-Change-Number: 19417
Gerrit-PatchSet: 24
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-Assignee: daniel <daniel at totalueberwachung.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: lynxis lazus <lynxis at fe80.eu>
Gerrit-CC: daniel <daniel at totalueberwachung.de>
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Wed, 09 Sep 2020 11:30:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: lynxis lazus <lynxis at fe80.eu>
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/20200909/88e00c6a/attachment.htm>


More information about the gerrit-log mailing list