Hi Pablo
* 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)
This is a bad practise. Leaving code that has been commented is not of
any help, it distracts the attention of the reader (who would wonder why
it's still there). On the other hand, if the intention is to track
changes, the SCM already does this for you, with it it's easy to see
what code was removed and added along time. If the intention to leave
that code commented to remark any special (or tricky) case, better add
some comment on that (my criteria usually is to document special cases
that I would even forget myself).
Sry for the misunderstanding question, but my question was why the "//"
comments are not used.
BTW, adding a short explanation to the patch also helps a lot to apply
them with git-am.
Thx for this hint. The patches were created with the command "git diff
HEAD > sms_openbsc.patch". But now i know that was not the correct way.
Now i have created a branch, commited the changes (with a comment) and
created the patch with "git format-patch master --stdout >
sms_openbsc.patch".
Enclosed are the new patches.
Best Regards
Dennis
From deb782ae35f8276609458ec9e9fb4add70b64614 Mon Sep 17 00:00:00 2001
From: Dennis Wehrle <[email protected]>
Date: Tue, 22 Mar 2011 18:18:14 +0100
Subject: [PATCH] SMS-fix: due to a misuse of user_data_len the sms where
cropped (on vty) and wrongly stored on the database.
---
openbsc/src/libmsc/db.c | 22 ++++++++++++++--------
openbsc/src/libmsc/gsm_04_11.c | 4 +---
openbsc/src/libmsc/gsm_04_80.c | 4 +++-
3 files changed, 18 insertions(+), 12 deletions(-)
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);
--
1.7.1
From 9a35914bc1c7b78d13ec400350eb8639e8125a0e Mon Sep 17 00:00:00 2001
From: Dennis Wehrle <[email protected]>
Date: Tue, 22 Mar 2011 18:25:53 +0100
Subject: [PATCH] SMS-fix: due to a misuse of user_data_len the sms where
cropped (on vty) and wrongly stored on the database.
Therefore two new functions was created: uint8_t gsm_get_octet_len(const
uint8_t sept_len) and uint8_t gsm_get_septet_len(const char *text)
---
include/osmocore/gsm_utils.h | 3 ++
src/gsm_utils.c | 54 ++++++++++++++++++++++++++++++++++++++---
2 files changed, 53 insertions(+), 4 deletions(-)
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
--
1.7.1
