4 Oct
2013
4 Oct
'13
2:42 a.m.
This patch set is mostly minor fixes and clean ups, with the exception of
the last two patches ("subscriber create" VTY command and a standard way for
printing a subscriber's info).
I would appreciate merging this patches to master, as I'm working onmore code
which depends on them.
Alexander Chemeris (6):
Fix copy-paste error in console output in db_test.
Fix typo ',' -> ';' at the end of a line.
Fix typo in console output: "PEROIDOC" -> "PERIODIC".
Slight clean up of the code in gsm340_rx_tpdu() and a fix for an
unlikely, but possible memory leak there.
Add "subscriber create" VTY command.
Introduce a standard way for printing a subscriber's info.
openbsc/include/openbsc/gsm_subscriber.h | 4 ++++
openbsc/src/libmsc/db.c | 2 +-
openbsc/src/libmsc/gsm_04_08.c | 2 +-
openbsc/src/libmsc/gsm_04_11.c | 7 ++++---
openbsc/src/libmsc/vty_interface_layer3.c | 27 +++++++++++++++++++++++++++
openbsc/tests/db/db_test.c | 2 +-
6 files changed, 38 insertions(+), 6 deletions(-)
--
1.7.9.5
4 Oct
4 Oct
2:42 a.m.
New subject: [PATCH 1/6] Fix copy-paste error in console output in db_test.
---
openbsc/tests/db/db_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/openbsc/tests/db/db_test.c b/openbsc/tests/db/db_test.c
index c0ee618..c3beee2 100644
--- a/openbsc/tests/db/db_test.c
+++ b/openbsc/tests/db/db_test.c
@@ -54,7 +54,7 @@ static struct gsm_network dummy_net;
printf("names do not match in %s:%d '%s' '%s'\n", \
__FUNCTION__, __LINE__, original->name, copy->name); \
if (strcmp(original->extension, copy->extension) != 0) \
- printf("names do not match in %s:%d '%s' '%s'\n", \
+ printf("Extensions do not match in %s:%d '%s' '%s'\n", \
__FUNCTION__, __LINE__, original->extension, copy->extension); \
int main()
--
1.7.9.5
2:42 a.m.
New subject: [PATCH 2/6] Fix typo ',' -> ';' at the end of a line.
Funny, this is a correct C expression and doesn't change execution, thus it
stayed unnoticed for quite a while.
---
openbsc/src/libmsc/db.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c
index 4e20e23..12d60bb 100644
--- a/openbsc/src/libmsc/db.c
+++ b/openbsc/src/libmsc/db.c
@@ -996,7 +996,7 @@ int db_subscriber_assoc_imei(struct gsm_subscriber *subscriber, char
imei[GSM_IM
dbi_result result;
strncpy(subscriber->equipment.imei, imei,
- sizeof(subscriber->equipment.imei)-1),
+ sizeof(subscriber->equipment.imei)-1);
result = dbi_conn_queryf(conn,
"INSERT OR IGNORE INTO Equipment "
--
1.7.9.5
2:42 a.m.
New subject: [PATCH 3/6] Fix typo in console output: "PEROIDOC" -> "PERIODIC".
---
openbsc/src/libmsc/gsm_04_08.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c
index cd31f69..f771b02 100644
--- a/openbsc/src/libmsc/gsm_04_08.c
+++ b/openbsc/src/libmsc/gsm_04_08.c
@@ -513,7 +513,7 @@ static const char *lupd_name(uint8_t type)
case GSM48_LUPD_NORMAL:
return "NORMAL";
case GSM48_LUPD_PERIODIC:
- return "PEROIDOC";
+ return "PERIODIC";
case GSM48_LUPD_IMSI_ATT:
return "IMSI ATTACH";
default:
--
1.7.9.5
2:42 a.m.
New subject: [PATCH 4/6] Slight clean up of the code in gsm340_rx_tpdu() and a fix for an unlikely, but possible memory leak there.
---
openbsc/src/libmsc/gsm_04_11.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index 2fc250b..e554b74 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -337,8 +337,8 @@ static int gsm340_rx_tpdu(struct gsm_subscriber_connection *conn,
struct msgb *m
sms_alphabet = gsm338_get_sms_alphabet(gsms->data_coding_scheme);
if (sms_alphabet == 0xffffffff) {
- sms_free(gsms);
- return GSM411_RP_CAUSE_MO_NET_OUT_OF_ORDER;
+ rc = GSM411_RP_CAUSE_MO_NET_OUT_OF_ORDER;
+ goto out;
}
switch (sms_vpf) {
@@ -359,7 +359,8 @@ static int gsm340_rx_tpdu(struct gsm_subscriber_connection *conn,
struct msgb *m
default:
LOGP(DLSMS, LOGL_NOTICE,
"SMS Validity period not implemented: 0x%02x\n", sms_vpf);
- return GSM411_RP_CAUSE_MO_NET_OUT_OF_ORDER;
+ rc = GSM411_RP_CAUSE_MO_NET_OUT_OF_ORDER;
+ goto out;
}
gsms->user_data_len = *smsp++;
if (gsms->user_data_len) {
--
1.7.9.5
2:42 a.m.
New subject: [PATCH 5/6] Add "subscriber create" VTY command.
It may be useful in production, but it's really required for VTY testing of
subscriber related commands.
---
openbsc/src/libmsc/vty_interface_layer3.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/openbsc/src/libmsc/vty_interface_layer3.c
b/openbsc/src/libmsc/vty_interface_layer3.c
index 8dc2812..dd657fb 100644
--- a/openbsc/src/libmsc/vty_interface_layer3.c
+++ b/openbsc/src/libmsc/vty_interface_layer3.c
@@ -210,6 +210,32 @@ DEFUN(show_subscr,
return CMD_SUCCESS;
}
+DEFUN(subscriber_create,
+ subscriber_create_cmd,
+ "subscriber create imsi ID",
+ "Operations on a Subscriber\n" \
+ "Create new subscriber\n" \
+ "Identify the subscriber by his IMSI\n" \
+ "Identifier for the subscriber\n")
+{
+ struct gsm_network *gsmnet = gsmnet_from_vty(vty);
+ struct gsm_subscriber *subscr;
+
+ subscr = db_create_subscriber(gsmnet, argv[0]);
+ if (!subscr) {
+ vty_out(vty, "%% No subscriber created for IMSI %s%s",
+ argv[0], VTY_NEWLINE);
+ return CMD_WARNING;
+ }
+
+ /* Show info about the created subscriber. */
+ subscr_dump_full_vty(vty, subscr, 0);
+
+ subscr_put(subscr);
+
+ return CMD_SUCCESS;
+}
+
DEFUN(subscriber_send_pending_sms,
subscriber_send_pending_sms_cmd,
"subscriber " SUBSCR_TYPES " ID sms pending-send",
@@ -934,6 +960,7 @@ int bsc_vty_init_extra(void)
install_element_ve(&sms_send_pend_cmd);
+ install_element_ve(&subscriber_create_cmd);
install_element_ve(&subscriber_send_sms_cmd);
install_element_ve(&subscriber_silent_sms_cmd);
install_element_ve(&subscriber_silent_call_start_cmd);
--
1.7.9.5
8:22 a.m.
New subject: [PATCH 5/6] Add "subscriber create" VTY command.
On Fri, Oct 04, 2013 at 02:42:28AM +0200, Alexander Chemeris wrote:
It may be useful in production, but it's really
required for VTY testing of
subscriber related commands.
Hi,
nice patch. Please create a VTY unit test for this.
holger
12:34 p.m.
New subject: [PATCH 5/6] Add "subscriber create" VTY command.
On Fri, Oct 4, 2013 at 2:22 AM, Holger Hans Peter Freyther
<holger(a)freyther.de> wrote:
On Fri, Oct 04, 2013 at 02:42:28AM +0200, Alexander Chemeris wrote:
Yeah, I was thinking about that, but haven't come up with a good set
of things to do. What would you propose to test in the VTY test here?
The only thing I could think of is issuing "subscriber create" and
then "show subscriber" and checking that they show the same.
Btw, what do you think about having "subscriber show" command as a
alias for "show subscriber" command?
--
Regards,
Alexander Chemeris.
CEO, Fairwaves LLC / ООО УмРадио
http://fairwaves.ru
It may be useful in production, but it's
really required for VTY testing of
subscriber related commands.
nice patch. Please create a VTY unit test for this.
1:25 p.m.
New subject: [PATCH 5/6] Add "subscriber create" VTY command.
On Fri, Oct 04, 2013 at 06:34:31AM -0400, Alexander Chemeris wrote:
Yeah, I was thinking about that, but haven't come
up with a good set
of things to do. What would you propose to test in the VTY test here?
The only thing I could think of is issuing "subscriber create" and
then "show subscriber" and checking that they show the same.
yes, that is all. show subscriber imsi 12345.. see that it fails,
create the subscriber, do the show again and see it is working.
2:41 p.m.
New subject: [PATCH 5/6] Add "subscriber create" VTY command.
On Fri, Oct 4, 2013 at 1:25 PM, Holger Hans Peter Freyther
<holger(a)freyther.de> wrote:
On Fri, Oct 04, 2013 at 06:34:31AM -0400, Alexander
Chemeris wrote:
Ok, sounds good, I'll do that.
--
Regards,
Alexander Chemeris.
CEO, Fairwaves LLC / ООО УмРадио
http://fairwaves.ru
Yeah, I was thinking about that, but haven't
come up with a good set
of things to do. What would you propose to test in the VTY test here?
The only thing I could think of is issuing "subscriber create" and
then "show subscriber" and checking that they show the same.
yes, that is all. show subscriber imsi 12345.. see that it fails,
create the subscriber, do the show again and see it is working.
2:42 a.m.
New subject: [PATCH 6/6] Introduce a standard way for printing a subscriber's info.
The intention is to standardize subscriber identification output in log files.
At this moment we sometimes identify subscriber by id, sometimes by extension,
sometimes by IMSI and sometimes do not idenify at all. This makes log analysis
hard or impossible.
This patch doesn't touch actual log output, as it's a separate thing and
requires consideration before checking in.
---
openbsc/include/openbsc/gsm_subscriber.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/openbsc/include/openbsc/gsm_subscriber.h
b/openbsc/include/openbsc/gsm_subscriber.h
index 78f9710..f21b3e6 100644
--- a/openbsc/include/openbsc/gsm_subscriber.h
+++ b/openbsc/include/openbsc/gsm_subscriber.h
@@ -15,6 +15,10 @@
#define GSM_SUBSCRIBER_FIRST_CONTACT 0x00000001
#define tmsi_from_string(str) strtoul(str, NULL, 10)
+#define GSM_SUBS_FMT_STR "IMSI %s (id %llu, ext %s%s%s)"
+#define GSM_SUBS_FMT_VAL(x) (x)->imsi, (x)->id, (x)->extension, \
+ strlen((x)->name)?", name ":"",
strlen((x)->name)?(x)->name:""
+
struct vty;
struct gsm_equipment {
--
1.7.9.5
8:17 a.m.
New subject: [PATCH 6/6] Introduce a standard way for printing a subscriber's info.
On Fri, Oct 04, 2013 at 02:42:29AM +0200, Alexander Chemeris wrote:
This patch doesn't touch actual log output, as
it's a separate thing and
requires consideration before checking in.
I will merge this once there is an user.
12:48 p.m.
New subject: [PATCH 6/6] Introduce a standard way for printing a subscriber's info.
On Fri, Oct 4, 2013 at 8:17 AM, Holger Hans Peter Freyther
<holger(a)freyther.de> wrote:
On Fri, Oct 04, 2013 at 02:42:29AM +0200, Alexander
Chemeris wrote:
Ok, I'll re-submit this as a part of another series of patches. I
wanted to get some review before I start using this widely.
--
Regards,
Alexander Chemeris.
CEO, Fairwaves LLC / ООО УмРадио
http://fairwaves.ru
This patch doesn't touch actual log output,
as it's a separate thing and
requires consideration before checking in.
I will merge this once there is an user. 
8:58 a.m.
New subject: [PATCH 6/6] Introduce a standard way for printing a subscriber's info.
Hi,
Two cents:
On Fri, Oct 4, 2013 at 3:42 AM, Alexander Chemeris
<alexander.chemeris(a)gmail.com> wrote:
+#define GSM_SUBS_FMT_STR "IMSI %s (id %llu, ext
%s%s%s)"
+#define GSM_SUBS_FMT_VAL(x) (x)->imsi, (x)->id, (x)->extension, \
+ strlen((x)->name)?", name ":"",
strlen((x)->name)?(x)->name:""
Checking for an empty string might be done by testing *s or s[0]
instead of strlen(s).
Also the second check for an empty string is superfluous, since it'll
print the same either way.
So one might write that as:
#define GSM_SUBS_FMT_VAL(x) (x)->imsi, (x)->id, (x)->extension, \
(x)->name[0] ? ", name " : "", (x)->name
Cheers,
Alex
12:46 p.m.
New subject: [PATCH 6/6] Introduce a standard way for printing a subscriber's info.
Alex,
On Fri, Oct 4, 2013 at 8:58 AM, Alex Badea <vamposdecampos(a)gmail.com> wrote:
On Fri, Oct 4, 2013 at 3:42 AM, Alexander Chemeris
<alexander.chemeris(a)gmail.com> wrote:
I was thinking about this and decided for strlen(), because it's more
human readable. But on the second look I think s[0] is not too bad.
Thank you for bringing this up.
+#define GSM_SUBS_FMT_STR "IMSI %s (id %llu,
ext %s%s%s)"
+#define GSM_SUBS_FMT_VAL(x) (x)->imsi, (x)->id, (x)->extension, \
+ strlen((x)->name)?", name ":"",
strlen((x)->name)?(x)->name:""
Checking for an empty string might be done by testing *s or s[0]
instead of strlen(s). Also the second check for an empty string is
superfluous, since it'll
print the same either way.
Good catch, thanks.
So one might write that as:
#define GSM_SUBS_FMT_VAL(x) (x)->imsi, (x)->id, (x)->extension, \
(x)->name[0] ? ", name " : "", (x)->name
I'll use this for the second iteration of the patch.
--
Regards,
Alexander Chemeris.
CEO, Fairwaves LLC / ООО УмРадио
http://fairwaves.ru
3868
days inactive
3868
days old
14 comments
3 participants
participants (3)
-
Alex Badea -
Alexander Chemeris -
Holger Hans Peter Freyther