Re: [PATCH 1/4] Internal and Driver API changes for Send USSD
Hi Jeevaka,
On 09/14/2010 04:23 PM, Jeevaka Badrappan wrote:
---
drivers/atmodem/ussd.c | 115 ++++++++++++++++++++++++++++-------------------
drivers/isimodem/ussd.c | 42 +++--------------
include/ussd.h | 8 ++-
src/ussd.c | 39 +++++++++++++---
4 files changed, 115 insertions(+), 89 deletions(-)
This patch has been applied, but refactored heavily afterward.
+ struct ussd_data *data = ofono_ussd_get_data(ussd);
GAtResultIter iter;
int status;
int dcs;
- const char *content;
- char *converted = NULL;
- gboolean udhi;
+ const char *content = NULL;
enum sms_charset charset;
- gboolean compressed;
- gboolean iso639;
+ unsigned char msg[160];
+ const unsigned char *msg_ptr = NULL;
+ unsigned char *converted = NULL;
+ long written;
+ long msg_len = 0;
Please avoid initializing variables unless absolutely necessary. This
actually hid some serious issues in the code which I had to fix. In the
future I will not be so nice and simply reject such patches.
+ switch (charset) {
+ case SMS_CHARSET_7BIT:
+ if (data->charset == AT_UTIL_CHARSET_GSM)
+ msg_ptr = pack_7bit_own_buf((const guint8 *) content,
+ strlen(content), 0, TRUE, &msg_len, 0, msg);
+ else if (data->charset == AT_UTIL_CHARSET_UTF8)
+ ussd_encode(content, &msg_len, msg);
+ else if (data->charset == AT_UTIL_CHARSET_UCS2) {
+ msg_ptr = decode_hex_own_buf(content, -1, &msg_len, 0, msg);
Please watch out for buffer overflows and make sure to run valgrind and
test all your code. msg in this case was too small to contain the
decoded UCS2 information (up to 182 * 2 characters)
+ snprintf(buf, sizeof(buf),
"AT+CUSD=1,\"%.*s\",%d",
+ (int) written, unpacked_buf, dcs);
+ } else {
+ converted = encode_hex_own_buf(pdu, len, 0, coded_buf);
Same thing here, you were overflowing the coded_buf
+ if (!converted)
+ goto error;
+
+ snprintf(buf, sizeof(buf), "AT+CUSD=1,\"%.*s\",%d",
+ strlen(converted), converted, dcs);
and overflowing buf as well
Regards,
-Denis