Change in libosmocore[master]: add osmo_gsup_make_response() and osmo_gsup_message_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/.

neels gerrit-no-reply at lists.osmocom.org
Thu Nov 28 21:00:30 UTC 2019


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

Change subject: add osmo_gsup_make_response() and osmo_gsup_message_name_*()
......................................................................


Patch Set 3:

(3 comments)

https://gerrit.osmocom.org/c/libosmocore/+/16189/3/src/gsm/gsup.c 
File src/gsm/gsup.c:

https://gerrit.osmocom.org/c/libosmocore/+/16189/3/src/gsm/gsup.c@910 
PS3, Line 910:  * Note: after calling this function, fields in the reply may reference the same memory as rx and are not deep-copied.
> Ack
that is what the osmo_gsup_req is for: it keeps the incoming message's state.
This non-deep copying of gsup messages is happening all over the osmo-hlr and gsup client code base.
We always keep the msgb around until the contents no longer matter.

Note that a struct osmo_gsup_message is so far never dynamically allocated, anywhere.
So if we wanted to deep-copy with dynamic allocations, we would open up a whole new refactoring slur all over the place. It's one of the things we should have designed better from the start, too late now in my opinion.

The solution to allow caching (decoded) GSUP messages also asynchronously is the osmo_gsup_req API added in osmo-hlr.git, in libosmo-gsupclient. So far we never kept decoded GSUP asynchronously, but will do for D-GSM, and btw also strictly require this in order for proxy routing to be guaranteed to work: to copy the right routing information back to the response.

Given that it's close to the IE layer, I thought here is the best place, but I'm going to move this into osmo_gsup_req now. You (I?) have just convinced me.


https://gerrit.osmocom.org/c/libosmocore/+/16189/3/src/gsm/gsup.c@943 
PS3, Line 943: 	if (!reply->imsi[0])
> Ack
We almost nowhere do explicit '\0' char matching. I used to do that everywhere some years ago, but the million places where we just use '!' made me lose interest. I'd prefer to spend my time differently than reworking many patches for cosmetics. Functional flaws are much nicer to discuss, like above.


https://gerrit.osmocom.org/c/libosmocore/+/16189/3/src/gsm/gsup.c@985 
PS3, Line 985: osmo_gsup_message_name_buf
> _stringify()? _tostr() _sprint() ? […]
I do see what you mean.

(for me '_name' turned out to be the API naming for "convert to human readable string", compare also OSMO_NAME_C_IMPL(), which then could have been OSMO_STR_C_IMPL() instead... I think we can still rename it, not released yet, is it? I'm fairly sure there are many places where I called functions foo_name even though they produce more than just a short name, so keep an eye out. OTOH this is another minor cosmetic up for interpretation...)



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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id9692880079ea0f219f52d81b1923a76fc640566
Gerrit-Change-Number: 16189
Gerrit-PatchSet: 3
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Thu, 28 Nov 2019 21:00:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin at sysmocom.de>
Comment-In-Reply-To: neels <nhofmeyr at sysmocom.de>
Comment-In-Reply-To: laforge <laforge at osmocom.org>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191128/ea7f9b40/attachment.htm>


More information about the gerrit-log mailing list