Changes made for the most part.One issue: How might one determine if the SMSC is offline (#7 below) and as such hold pending messages? Because the message parts could have come from different physical connections, does this mean we check each smsc connection of each part of the message before we discard?? Seems messy.
I've left that out for now.
concat.diff
Description: Binary data
On Mar 21, 2007, at 15:33, Alexander Malysh wrote:
Hi Paul, sorry for the delay, here we go with my comments... 1) fix indentation. We don't use tabs use 4 spaces instead 2) I don't think we want to see such things in access log:+ bb_alog_sms(conn, sms, "CONCATENATED SMS Part Received, stored"); You should just log the concatenated sms after reassembling or each part ifreassembling failed. 3) why do you use RWLock: +static RWLock *concat_rwlock; I see only that you need mutex instead of rwlock. 4) in the init_concat_handler you should init: + incoming_concat_msgs = dict_create(1024, destroy_concatMsg);based on incoming queue length. Otherwise it's possible that Dict will bevery slow.5) this here should be warning or error, because user want to know what'swrong:+ debug("concat.sms",0, "Duplicate message part %d, ref %d, from %s.Discarded!", + part, refnum, octstr_get_cstr(msg->sms.sender)); 6) deadlock (goto done with rwlock locked):+ /* Attempt to save the new one, if that fails, then reply with fail.*/ + if (store_save(msg) == -1) { + msg_destroy(msg); + *pmsg = msg = NULL; + ret = concat_error; + goto done; + } else + *pmsg = msg; /* return the message part. */7) removing messages from the list that are too old should go to smsc2_route because if you don't receive MOs then you will never clear list and it's slow down MO handling. Here you should take into account that SMSC may bejust offline so you don't want to drop those messages from the list. 8) please don't use: + dict_put(incoming_concat_msgs, key, NULL); /* delete it. */ use dict_remove instead. 9) please also supply userguide patch Paul Bagyenda wrote:Following your comments, here is a MO concatenation patch that now resides in bb_smscconn.c, and uses the key as advised below. Same code, largely, just integrated in a different place. There is also better handling of the message stores... Tests, comments, votes. P. Ps. If you are using the smsc_at2 module, be sure to patch with additional patch below.-- Thanks, Alex
