Hi Holger
On 19.03.2011 01:54, Holger Hans Peter Freyther wrote:
On 03/18/2011 11:01 PM, Dennis Wehrle wrote:
Hello
Hi Dennis,
thank you very much for your patch. There are some formal issues with the
patch and it would be nice if you could resolve them.
sure ;-)
* In general we don't keep commented out code around, please remove it.
* We avoid the // comments.
Can you explain why? (just for my interest)
* When introducing a new function that has known static linkage please
use a namespace, e.g. not get_octlet_length but at least
gsm_get_octet_length or put in 7bit as well.
Besides that your patch looks nice, but I do have two concerns. Your 160 char
limit assumes the default GSM alphabet? In the future we might be able to
encode and decode from different (e.g. chinese/unicode) alphabet. The same
concern is for the new code in db.c that handles things specially. It would be
nice if all of this could be hidden in libosmocore or such.
I have moved the septet length lookup to the gsm_utils.c. This function
looks for characters defined at the "GSM 7bit default alphabet extension
table" (GSM 03.38 6.2.1.1) which needs two septets. These characters
are: ^ | € { } [ ] ~ \
cheers
holger
Best Regards
Dennis
diff --git a/include/osmocore/gsm_utils.h b/include/osmocore/gsm_utils.h
index 0aadd2e..a47ce7b 100644
--- a/include/osmocore/gsm_utils.h
+++ b/include/osmocore/gsm_utils.h
@@ -59,6 +59,9 @@ enum gsm_band gsm_band_parse(const char *mhz);
int gsm_7bit_decode(char *decoded, const uint8_t *user_data, uint8_t length);
int gsm_7bit_encode(uint8_t *result, const char *data);
+uint8_t gsm_get_octet_len(const uint8_t sept_len);
+uint8_t gsm_get_septet_len(const char *text);
+
int ms_pwr_ctl_lvl(enum gsm_band band, unsigned int dbm);
int ms_pwr_dbm(enum gsm_band band, uint8_t lvl);
diff --git a/src/gsm_utils.c b/src/gsm_utils.c
index 31e3cd6..faac7cf 100644
--- a/src/gsm_utils.c
+++ b/src/gsm_utils.c
@@ -73,17 +73,52 @@ static int gsm_septet_lookup(uint8_t ch)
return -1;
}
+/* Warning, user_data_len indicates the amount of septets (characters), we need amount of octets occupied */
+/* compute the number of octets: */
+/* #octets = (#septets * 7 bit) / 8 bit */
+/* e.g. 47 non-extension characters needs 329 bits / 8 bit = 41,125 octets => we need 42 octets for 47 characters */
+uint8_t gsm_get_octet_len(const uint8_t sept_len){
+ int octet_len = (sept_len * 7) / 8;
+ if ((sept_len * 7) % 8 != 0)
+ octet_len++;
+
+ return octet_len;
+}
+
+
+/* Get number of septets. Each extension character has to be count twice, because they need 2 septets*/
+uint8_t gsm_get_septet_len(const char *text){
+ uint8_t septet_len = strlen(text);
+ int i;
+ for(i = 0; i < strlen(text); i++){
+ switch(text[i]){
+ case 0x0c:
+ case 0x5e:
+ case 0x7b:
+ case 0x7d:
+ case 0x5c:
+ case 0x5b:
+ case 0x7e:
+ case 0x5d:
+ case 0x7c:
+ septet_len++;
+ default:
+ continue;
+ }
+ }
+ return septet_len;
+}
+
/* GSM 03.38 6.2.1 Character unpacking */
int gsm_7bit_decode(char *text, const uint8_t *user_data, uint8_t length)
{
int i = 0;
- int l = 0;
- int septet_l = (length * 8) / 7;
+
+ int septet_l = length;
uint8_t *rtext = calloc(septet_l, sizeof(uint8_t));
uint8_t tmp;
/* FIXME: We need to account for user data headers here */
- i += l;
for (; i < septet_l; i++){
rtext[i] =
((user_data[(i * 7 + 7) >> 3] <<
@@ -155,6 +190,12 @@ int gsm_7bit_encode(uint8_t *result, const char *data)
shift = 0;
continue;
}
+ /* special end case. This is necessary if the last septet fits into the previous octet. */
+ /* E.g. 48 non-extension characters: ....ag ( a = 1100001, g = 1100111) */
+ /* result[40] = 100001 XX */
+ /* result[41] = 1100111 1 */
+ if(shift == 7 && i + 1 == y)
+ break;
cb = (rdata[i] & 0x7f) >> shift;
if(i + 1 < y){
@@ -168,7 +209,12 @@ int gsm_7bit_encode(uint8_t *result, const char *data)
}
free(rdata);
- return z;
+
+ /* we don't care about the number of octets, because they are not unique. E.g.: */
+ /* 1.) 46 non-extension characters + 1 extension character => (46 * 7 bit + (1 * (2 * 7 bit))) / 8 bit = 42 octets */
+ /* 2.) 47 non-extension characters => (47 * 7 bit) / 8 bit = 41,125 = 42 octets */
+ /* 3.) 48 non-extension characters => (48 * 7 bit) / 8 bit = 42 octects */
+ return y;
}
/* determine power control level for given dBm value, as indicated
diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c
index 95a7d36..cf72332 100644
--- a/openbsc/src/libmsc/db.c
+++ b/openbsc/src/libmsc/db.c
@@ -996,8 +996,10 @@ int db_sms_store(struct gsm_sms *sms)
dbi_conn_quote_string_copy(conn, (char *)sms->text, &q_text);
dbi_conn_quote_string_copy(conn, (char *)sms->dest_addr, &q_daddr);
- dbi_conn_quote_binary_copy(conn, sms->user_data, sms->user_data_len,
- &q_udata);
+
+ int octet_len = gsm_get_octet_len(sms->user_data_len);
+ dbi_conn_quote_binary_copy(conn, sms->user_data, octet_len, &q_udata);
+
/* FIXME: correct validity period */
result = dbi_conn_queryf(conn,
"INSERT INTO SMS "
@@ -1057,17 +1059,21 @@ static struct gsm_sms *sms_from_result(struct gsm_network *net, dbi_result resul
sms->dest_addr[sizeof(sms->dest_addr)-1] = '\0';
}
- sms->user_data_len = dbi_result_get_field_length(result, "user_data");
- user_data = dbi_result_get_binary(result, "user_data");
- if (sms->user_data_len > sizeof(sms->user_data))
- sms->user_data_len = (u_int8_t) sizeof(sms->user_data);
- memcpy(sms->user_data, user_data, sms->user_data_len);
-
text = dbi_result_get_string(result, "text");
if (text) {
strncpy(sms->text, text, sizeof(sms->text));
sms->text[sizeof(sms->text)-1] = '\0';
}
+
+ /* user_data_len contains the number of septets, therefore use the "text"-field to get user_data_len. */
+ /* But each extension character has to be count twice, because they need 2 septets. */
+ sms->user_data_len = gsm_get_septet_len(text);
+
+ user_data = dbi_result_get_binary(result, "user_data");
+ if (sms->user_data_len > sizeof(sms->user_data))
+ sms->user_data_len = (u_int8_t) sizeof(sms->user_data);
+ memcpy(sms->user_data, user_data, sms->user_data_len);
+
return sms;
}
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index 812e758..891571f 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -526,9 +526,7 @@ static int gsm340_gen_tpdu(struct msgb *msg, struct gsm_sms *sms)
/* generate TP-UD */
switch (gsm338_get_sms_alphabet(sms->data_coding_scheme)) {
case DCS_7BIT_DEFAULT:
- octet_len = sms->user_data_len*7/8;
- if (sms->user_data_len*7%8 != 0)
- octet_len++;
+ octet_len = gsm_get_octet_len(sms->user_data_len);
/* Warning, user_data_len indicates the amount of septets
* (characters), we need amount of octets occupied */
smsp = msgb_put(msg, octet_len);
diff --git a/openbsc/src/libmsc/gsm_04_80.c b/openbsc/src/libmsc/gsm_04_80.c
index 494c319..90227c7 100644
--- a/openbsc/src/libmsc/gsm_04_80.c
+++ b/openbsc/src/libmsc/gsm_04_80.c
@@ -73,7 +73,9 @@ int gsm0480_send_ussd_response(struct gsm_subscriber_connection *conn,
/* First put the payload text into the message */
ptr8 = msgb_put(msg, 0);
response_len = gsm_7bit_encode(ptr8, response_text);
- msgb_put(msg, response_len);
+
+ int octet_len = gsm_get_octet_len(response_len);
+ msgb_put(msg, octet_len);
/* Then wrap it as an Octet String */
msgb_wrap_with_TL(msg, ASN1_OCTET_STRING_TAG);
