Hello,
it looks like in the following piece of code (gw/smsc/smsc_emi.c:1080),
the adc var is useless :
----------------------------------------------------------------
if (octstr_get_char(emimsg->fields[0], 0) == 'A') {
/* we got an ack back. We might have to store the */
/* timestamp for delivery notifications now */
Octstr *ts, *adc;
int i;
Msg *m;
ts = octstr_duplicate(emimsg->fields[2]);
if (octstr_len(ts)) {
i = octstr_search_char(ts,':',0);
if (i>0) {
octstr_delete(ts,0,i+1);
adc = octstr_duplicate(emimsg->fields[2]);
octstr_truncate(adc,i);
m = PRIVDATA(conn)->slots[emimsg->trn].sendmsg;
if(m == NULL) {
info(0,"EMI2[%s]: uhhh m is NULL, very bad",
octstr_get_cstr(privdata->name));
} else if (DLR_IS_ENABLED_DEVICE(m->sms.dlr_mask)) {
dlr_add((conn->id ? conn->id : privdata->name), ts, m);
}
octstr_destroy(ts);
octstr_destroy(adc);
} else {
octstr_destroy(ts);
}
}
----------------------------------------------------------------
I think that the adc variable is useless, as it's only used to retrieve
a part of emimsg->fields[2] and then it's destroyed without using it.
Another remark is that variables should be named with more explicit
names. Short names doesn't make the code faster, but it makes it harder
to read. In this example, maybe ts could be named timestamp, maybe m
could be named message etc ... It does'nt even allow to code faster, as
a comment has to explain that we store the timestamp.
--
Colin Pitrat (Bull Services Telco)
Bull, Architect of an Open World (TM)
Tél : +33 (0) 1 30 80 72 93
www.bull.com
--- gateway/gw/smsc/smsc_emi.c 2006-08-18 10:21:59.000000000 +0200
+++ gateway-new/gw/smsc/smsc_emi.c 2006-08-18 10:18:59.000000000 +0200
@@ -1080,7 +1080,7 @@
if (octstr_get_char(emimsg->fields[0], 0) == 'A') {
/* we got an ack back. We might have to store the */
/* timestamp for delivery notifications now */
- Octstr *ts, *adc;
+ Octstr *ts;
int i;
Msg *m;
@@ -1089,8 +1089,6 @@
i = octstr_search_char(ts,':',0);
if (i>0) {
octstr_delete(ts,0,i+1);
- adc = octstr_duplicate(emimsg->fields[2]);
- octstr_truncate(adc,i);
m = PRIVDATA(conn)->slots[emimsg->trn].sendmsg;
if(m == NULL) {
@@ -1100,7 +1098,6 @@
dlr_add((conn->id ? conn->id :
privdata->name), ts, m);
}
octstr_destroy(ts);
- octstr_destroy(adc);
} else {
octstr_destroy(ts);
}