pespin 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.
I see no deep-copied as a source of problems. Usually you want to queue this 
response message while freeing the rx message potentially beforehand (because 
theoretically the socket kernel queue could be full and we have our own wqueue).
Am I missing something in my picture?


https://gerrit.osmocom.org/c/libosmocore/+/16189/3/src/gsm/gsup.c@943
PS3, Line 943:  if (!reply->imsi[0])
I'll keep asking to please match against = '\0', it's then clear from reader 
point of view that this is a string and not some random pointer array.


https://gerrit.osmocom.org/c/libosmocore/+/16189/3/src/gsm/gsup.c@1040
PS3, Line 1040:         OSMO_NAME_C_IMPL(ctx, 64, "ERROR", 
osmo_gsup_message_name_buf, msg)
64 looks a bit small here given the amount of text above. 128?



--
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 <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: neels <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-Comment-Date: Wed, 27 Nov 2019 07:57:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to