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(a)gnumonks.org>
http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)