Change in osmo-hlr[master]: 1/2: refactor: add and use lu_fsm, osmo_gsup_req, osmo_ipa_name

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 Jan 8 13:14:29 UTC 2020


pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/16205 )

Change subject: 1/2: refactor: add and use lu_fsm, osmo_gsup_req, osmo_ipa_name
......................................................................


Patch Set 23:

(7 comments)

IMHO too many things going on in this commit which are not really related and which could have been easily splitted and discussed separately, like the use of ipa_name struct, te _req*, and other fixes in the way simply pulled in in the same patch.

https://gerrit.osmocom.org/c/osmo-hlr/+/16205/23/include/osmocom/gsupclient/ipa_name.h 
File include/osmocom/gsupclient/ipa_name.h:

https://gerrit.osmocom.org/c/osmo-hlr/+/16205/23/include/osmocom/gsupclient/ipa_name.h@29 
PS23, Line 29: struct osmo_ipa_name {
I'm not really liking this kind of adhoc structures to keep a string specially since they are presented in a public header of shared library, meaning we'll need to keep with it later on...


https://gerrit.osmocom.org/c/osmo-hlr/+/16205/23/include/osmocom/hlr/gsup_router.h 
File include/osmocom/hlr/gsup_router.h:

https://gerrit.osmocom.org/c/osmo-hlr/+/16205/23/include/osmocom/hlr/gsup_router.h@31 
PS23, Line 31: int osmo_gsup_send_to_ipa_name(struct osmo_gsup_server *gs, const struct osmo_ipa_name *ipa_name, struct msgb *msg);
Update TODO_RELEASE explaining there's new API.


https://gerrit.osmocom.org/c/osmo-hlr/+/16205/23/src/db.c 
File src/db.c:

https://gerrit.osmocom.org/c/osmo-hlr/+/16205/23/src/db.c@25 
PS23, Line 25: #include <errno.h>
Non related at all, must go into a separate patch (if needed at all).


https://gerrit.osmocom.org/c/osmo-hlr/+/16205/23/src/gsup_router.c 
File src/gsup_router.c:

https://gerrit.osmocom.org/c/osmo-hlr/+/16205/23/src/gsup_router.c@99 
PS23, Line 99: int gsup_route_add_ipa_name(struct osmo_gsup_conn *conn, const struct osmo_ipa_name *ipa_name)
and with this type of structs we end up doing like this function, simply adding extra layers.


https://gerrit.osmocom.org/c/osmo-hlr/+/16205/23/src/gsupclient/ipa_name.c 
File src/gsupclient/ipa_name.c:

https://gerrit.osmocom.org/c/osmo-hlr/+/16205/23/src/gsupclient/ipa_name.c@90 
PS23, Line 90: 	return osmo_escape_str_c(OTC_SELECT, (char*)ipa_name->val, len);
So you are forcing the app using the library to enable the otc_select in this case? SHouldn't the function have some kind of suffix to mark it as so (_c) and explain that in the API doc?


https://gerrit.osmocom.org/c/osmo-hlr/+/16205/23/src/hlr.c 
File src/hlr.c:

https://gerrit.osmocom.org/c/osmo-hlr/+/16205/23/src/hlr.c@426 
PS23, Line 426: 	     osmo_gsup_message_type_name((gsup)->message_type), \
Fixing () in the define could be a separate commit.


https://gerrit.osmocom.org/c/osmo-hlr/+/16205/23/src/hlr.c@777 
PS23, Line 777: 		osmo_select_main_ctx(0);
Is this line change only needed to print stuff more easily?



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

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Change-Id: I3a8dff3d4a1cbe10d6ab08257a0138d6b2a082d9
Gerrit-Change-Number: 16205
Gerrit-PatchSet: 23
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Wed, 08 Jan 2020 13:14:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200108/72b5e51f/attachment.htm>


More information about the gerrit-log mailing list