libosmocore[master]: GSUP: add USSD messages support according to 3GPP TS 09.02

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 Hofmeyr gerrit-no-reply at lists.osmocom.org
Wed Apr 18 16:10:46 UTC 2018


Patch Set 4:

(7 comments)

some suggestions...

https://gerrit.osmocom.org/#/c/7600/4/include/osmocom/gsm/gsup.h
File include/osmocom/gsm/gsup.h:

Line 97: 	/**
try to clarify which elements the doxygen comments refer to. Note that in the doxygen rendered API docs, only OSMO_GSUP_SS_INFO_IE will receive the comment you write here. Not sure who ever reads those instead of the code, but while we maintain doxygen API docs we might as well do it properly.

Also, we use /*! syntax.


Line 151: 	/**
doxygen again..


Line 246: 	uint32_t			invoke_id;
duplicate of the GSUP session id?


Line 252: 	const uint8_t			*ussd_string;
I don't suppose there is a maximum length of the string? if there were, we could use a uint8_t array and save the dynamic allocation.

Is it called "string" in the specs? if not, I'd prefer to not confuse the meaning with char*, i.e. rather use "data" than "string"


Line 254: 	const uint8_t			*ss_ap;
need a len for this? who owns the pointer target? what data is in here?


https://gerrit.osmocom.org/#/c/7600/4/src/gsm/gsup.c
File src/gsm/gsup.c:

Line 462: 			rc = decode_ss_info(value, value_len, &gsup_msg->ss_info);
are the SS info IEs packed inside a TLV? Wouldn't it be simpler to just have one flat layer of IEs?


Line 571: 			sizeof(uint8_t), ss_info->ss_ap);
either sizeof(ss_info->ss_ap) or just use 1. sizeof(uint8_t) is nonsense.


-- 
To view, visit https://gerrit.osmocom.org/7600
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17a78043a35fffbdd59e80fd2b2da39cce5e532
Gerrit-PatchSet: 4
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Reviewer: Alexander Chemeris <Alexander.Chemeris at gmail.com>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-HasComments: Yes



More information about the gerrit-log mailing list