Mobile-originated USSD - double D'oh!

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/OpenBSC@lists.osmocom.org/.

Harald Welte laforge at gnumonks.org
Fri Oct 16 06:53:31 UTC 2009


Hi Mike!

Some comments:

*  it would have been good to Cc the mailinglist when you re-posted the patch,
   this way more eyes are on the code
*  I've looked at it but unfortunately there's a number of issues.  That's
   why I've committed it to a new 'ussd' branch in git.
*  I've pushed an incremental patch as commit that cleans up the coding style
   issues that I had seen with the code

There are also issues with the actual code that I'd like to see addressed,
preferrably by incremental patches to my ussd branch:

* the use of static global variables like ussd_string_buf in gsm_04_80.c

Code like this will prove very difficult to ever use concurrently at some
later point.  global variables in the main program (or similar) are fine,
but in actual 'library' style code, we should not keep code in static global
variables across multiple calls.  In such a case, the state needs to be
associated with some of the data structures we have in memory anyway.

ussd_string_buff looks like something the caller should allocate and pass into
gsm0480_rcv_ussd().  Also, since rcv_ussd() only decodes, it should probably be
called gsm0480_decode_ussd().

Things like the static last_transaction_id also will cause race conditions once
multiple people send USSD requests at the same time.  This data needs to be per
subscriber, or per lchan, I presume.


* the assumption that extension numbers are not longer than 5 characters
  in ussd.c is really not good.  Please check the specs what is the maximum
  length of the phone number in this context, add a #define and then make sure
  the string buffer is large enough for it.  Also, I prefer snprintf() to
  having XXXX that then get overwritten by memcopy :)

If you could send patches for those two issues (one patch for each logical
change), I would appreciate that and in the end merge the ussd branch into
master.

Thanks.
-- 
- Harald Welte <laforge at gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)




More information about the OpenBSC mailing list