14 Jun
2013
14 Jun
'13
3:03 p.m.
Proposal for cseq:
cseq numbering:
--- a/modules/dialog/dlg_handlers.c Wed Apr 03 13:33:38 2013 +0200
+++ b/modules/dialog/dlg_handlers.c Fri Jun 14 13:39:47 2013 +0200
@@ -220,7 +220,7 @@
cseq = (get_cseq(msg))->number;
} else {
/* use the same as in request */
- cseq = dlg->cseq[DLG_CALLER_LEG];
+ cseq = dlg->cseq[DLG_CALLEE_LEG];
}
avoid memory leak:
--- a/modules/dialog/dlg_hash.c Fri Jun 14 13:40:12 2013 +0200
+++ b/modules/dialog/dlg_hash.c Fri Jun 14 13:45:21 2013 +0200
@@ -485,7 +485,14 @@
char *p;
dlg->tag[leg].s = (char*)shm_malloc( tag->len + rr->len +
contact->len );
- dlg->cseq[leg].s = (char*)shm_malloc( cseq->len );
+ if(dlg->cseq[leg].s){
+ if (dlg->cseq[leg].len < cseq->len) {
+ shm_free(dlg->cseq[leg].s);
+ dlg->cseq[leg].s = (char*)shm_malloc(cseq->len);
+ }
+ }else{
+ dlg->cseq[leg].s = (char*)shm_malloc( cseq->len );
+ }
if ( dlg->tag[leg].s==NULL || dlg->cseq[leg].s==NULL) {
LM_ERR("no more shm mem\n");
if (dlg->tag[leg].s)
18 Jun
18 Jun
11:50 a.m.
Hello,
thanks for the patches, I will review and commit as needed.
Cheers,
Daniel
On 6/14/13 2:03 PM, Halina Nowak wrote:
Proposal for cseq:
cseq numbering:
--- a/modules/dialog/dlg_handlers.c Wed Apr 03 13:33:38 2013 +0200
+++ b/modules/dialog/dlg_handlers.c Fri Jun 14 13:39:47 2013 +0200
@@ -220,7 +220,7 @@
cseq = (get_cseq(msg))->number;
} else {
/* use the same as in request */
- cseq = dlg->cseq[DLG_CALLER_LEG];
+ cseq = dlg->cseq[DLG_CALLEE_LEG];
}
avoid memory leak:
--- a/modules/dialog/dlg_hash.c Fri Jun 14 13:40:12 2013 +0200
+++ b/modules/dialog/dlg_hash.c Fri Jun 14 13:45:21 2013 +0200
@@ -485,7 +485,14 @@
char *p;
dlg->tag[leg].s = (char*)shm_malloc( tag->len + rr->len +
contact->len );
- dlg->cseq[leg].s = (char*)shm_malloc( cseq->len );
+ if(dlg->cseq[leg].s){
+ if (dlg->cseq[leg].len < cseq->len) {
+ shm_free(dlg->cseq[leg].s);
+ dlg->cseq[leg].s = (char*)shm_malloc(cseq->len);
+ }
+ }else{
+ dlg->cseq[leg].s = (char*)shm_malloc( cseq->len );
+ }
if ( dlg->tag[leg].s==NULL || dlg->cseq[leg].s==NULL) {
LM_ERR("no more shm mem\n");
if (dlg->tag[leg].s)
_______________________________________________
SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list
sr-users(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Kamailio Advanced Training, San Francisco, USA - June 24-27, 2013
* http://asipto.com/u/katu *
21 Jun
21 Jun
8:32 p.m.
Hello,
some comments inline.
On 6/14/13 2:03 PM, Halina Nowak wrote:
Proposal for cseq:
cseq numbering:
--- a/modules/dialog/dlg_handlers.c Wed Apr 03 13:33:38 2013 +0200
+++ b/modules/dialog/dlg_handlers.c Fri Jun 14 13:39:47 2013 +0200
@@ -220,7 +220,7 @@
cseq = (get_cseq(msg))->number;
} else {
/* use the same as in request */
- cseq = dlg->cseq[DLG_CALLER_LEG];
+ cseq = dlg->cseq[DLG_CALLEE_LEG];
at quick check, the comment says the cseq is taken from the request
because it is processing the reply in this part of the code.
You change that to take the value of the stored cseq for callee, which
is different than the current processed message.
What was the problem you discovered? Can you give an example to
understand this change?
}
avoid memory leak:
--- a/modules/dialog/dlg_hash.c Fri Jun 14 13:40:12 2013 +0200
+++ b/modules/dialog/dlg_hash.c Fri Jun 14 13:45:21 2013 +0200
@@ -485,7 +485,14 @@
char *p;
dlg->tag[leg].s = (char*)shm_malloc( tag->len + rr->len +
contact->len );
- dlg->cseq[leg].s = (char*)shm_malloc( cseq->len );
+ if(dlg->cseq[leg].s){
+ if (dlg->cseq[leg].len < cseq->len) {
+ shm_free(dlg->cseq[leg].s);
+ dlg->cseq[leg].s = (char*)shm_malloc(cseq->len);
+ }
+ }else{
+ dlg->cseq[leg].s = (char*)shm_malloc( cseq->len );
+ }
if ( dlg->tag[leg].s==NULL || dlg->cseq[leg].s==NULL) {
LM_ERR("no more shm mem\n");
if (dlg->tag[leg].s)
I see tag is also allocated each time, isn't
it exposed to same issue?
You changed only cseq to reuse existing buffer or free the old one and
allocate a bigger one when needed.
Cheers,
Daniel
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
24 Jun
24 Jun
3:39 p.m.
New subject: dialog: cseq + EARLY dialog
Hello,
These modifications as well as those for EARLY dialog were implemented
in the solution using PRACK and 2 UPDATEs (one in each direction )
before 200OK.
The cseq numbering was incorrect when server itself decided to cut the
communication and sent 2 BYEs.
The cseqs in UPDATEs where 2 and 22 and in BYEs they were 1 and 21
instead of 3 and 23, so BYEs were rejected by clients.
Always in this solution I detected memory leak on cseq, not on tag.
Maybe this memory leak was because of using UPDATEs - I'm not sure.
I've implemented these modifications 3 years ago and I've "found" them
porting our solution to kamailio4.
Cheers
Halina Nowak
On 06/21/2013 07:32 PM, Daniel-Constantin Mierla wrote:
Hello,
some comments inline.
On 6/14/13 2:03 PM, Halina Nowak wrote:
Proposal for cseq:
cseq numbering:
--- a/modules/dialog/dlg_handlers.c Wed Apr 03 13:33:38 2013 +0200
+++ b/modules/dialog/dlg_handlers.c Fri Jun 14 13:39:47 2013 +0200
@@ -220,7 +220,7 @@
cseq = (get_cseq(msg))->number;
} else {
/* use the same as in request */
- cseq = dlg->cseq[DLG_CALLER_LEG];
+ cseq = dlg->cseq[DLG_CALLEE_LEG];
at quick check, the comment says the cseq is taken from the request
because it is processing the reply in this part of the code.
You change that to take the value of the stored cseq for callee, which
is different than the current processed message.
What was the problem you discovered? Can you give an example to
understand this change?
}
avoid memory leak:
--- a/modules/dialog/dlg_hash.c Fri Jun 14 13:40:12 2013 +0200
+++ b/modules/dialog/dlg_hash.c Fri Jun 14 13:45:21 2013 +0200
@@ -485,7 +485,14 @@
char *p;
dlg->tag[leg].s = (char*)shm_malloc( tag->len + rr->len +
contact->len );
- dlg->cseq[leg].s = (char*)shm_malloc( cseq->len );
+ if(dlg->cseq[leg].s){
+ if (dlg->cseq[leg].len < cseq->len) {
+ shm_free(dlg->cseq[leg].s);
+ dlg->cseq[leg].s = (char*)shm_malloc(cseq->len);
+ }
+ }else{
+ dlg->cseq[leg].s = (char*)shm_malloc( cseq->len );
+ }
if ( dlg->tag[leg].s==NULL || dlg->cseq[leg].s==NULL) {
LM_ERR("no more shm mem\n");
if (dlg->tag[leg].s)
I see tag is also allocated each time, isn't
it exposed to same issue?
You changed only cseq to reuse existing buffer or free the old one and
allocate a bigger one when needed.
Cheers,
Daniel
--
Daniel-Constantin Mierla -http://www.asipto.com
http://twitter.com/#!/miconda -http://www.linkedin.com/in/miconda
25 Jun
25 Jun
8:09 a.m.
New subject: dialog: cseq + EARLY dialog
Hello,
On 6/24/13 2:39 PM, Halina Nowak wrote:
Hello,
These modifications as well as those for EARLY dialog were implemented
in the solution using PRACK and 2 UPDATEs (one in each direction )
before 200OK.
The cseq numbering was incorrect when server itself decided to cut the
communication and sent 2 BYEs.
The cseqs in UPDATEs where 2 and 22 and in BYEs they were 1 and 21
instead of 3 and 23, so BYEs were rejected by clients.
ok, updating the cseq should
happen for prack and update requests. But I
couldn't get the relation between the patches and fixing this problem.
Another thing was switching to state confirmed from early, not going
through confirmed_na (which is confirmend with no ack). This might be
due to the fact that your patch is rather old and maybe at that time was
no confirmed_na state.
Always in this solution I detected memory leak on cseq, not on tag.
Maybe this memory leak was because of using UPDATEs - I'm not sure.
Looked a little odd that in a function that allocated two chunks of
shared memory only one is being take care of. Because if that function
is executed many times, protection against a leak should be in both cases.
I've implemented these modifications 3 years ago
and I've "found" them
porting our solution to kamailio4.
What is the version you had the patches for?
just to check the
development of the module and see what changes were done in general.
Thanks,
Daniel
Cheers
Halina Nowak
On 06/21/2013 07:32 PM, Daniel-Constantin Mierla wrote:
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Hello,
some comments inline.
On 6/14/13 2:03 PM, Halina Nowak wrote:
Proposal for cseq:
cseq numbering:
--- a/modules/dialog/dlg_handlers.c Wed Apr 03 13:33:38 2013 +0200
+++ b/modules/dialog/dlg_handlers.c Fri Jun 14 13:39:47 2013 +0200
@@ -220,7 +220,7 @@
cseq = (get_cseq(msg))->number;
} else {
/* use the same as in request */
- cseq = dlg->cseq[DLG_CALLER_LEG];
+ cseq = dlg->cseq[DLG_CALLEE_LEG];
at quick check, the comment says the cseq is taken from the request
because it is processing the reply in this part of the code.
You change that to take the value of the stored cseq for callee,
which is different than the current processed message.
What was the problem you discovered? Can you give an example to
understand this change?
}
avoid memory leak:
--- a/modules/dialog/dlg_hash.c Fri Jun 14 13:40:12 2013 +0200
+++ b/modules/dialog/dlg_hash.c Fri Jun 14 13:45:21 2013 +0200
@@ -485,7 +485,14 @@
char *p;
dlg->tag[leg].s = (char*)shm_malloc( tag->len + rr->len +
contact->len );
- dlg->cseq[leg].s = (char*)shm_malloc( cseq->len );
+ if(dlg->cseq[leg].s){
+ if (dlg->cseq[leg].len < cseq->len) {
+ shm_free(dlg->cseq[leg].s);
+ dlg->cseq[leg].s = (char*)shm_malloc(cseq->len);
+ }
+ }else{
+ dlg->cseq[leg].s = (char*)shm_malloc( cseq->len );
+ }
if ( dlg->tag[leg].s==NULL || dlg->cseq[leg].s==NULL) {
LM_ERR("no more shm mem\n");
if (dlg->tag[leg].s)
I see tag is also allocated each time, isn't
it exposed to same
issue? You changed only cseq to reuse existing buffer or free the old
one and allocate a bigger one when needed.
Cheers,
Daniel
--
Daniel-Constantin Mierla -http://www.asipto.com
http://twitter.com/#!/miconda -http://www.linkedin.com/in/miconda 
1 Jul
1 Jul
6:05 a.m.
Hbhj av
On 14 Jun 2013 17:33, "Halina Nowak" <halina(a)mbdsys.com> wrote:
Proposal for cseq:
cseq numbering:
--- a/modules/dialog/dlg_handlers.c Wed Apr 03 13:33:38 2013 +0200
+++ b/modules/dialog/dlg_handlers.c Fri Jun 14 13:39:47 2013 +0200
@@ -220,7 +220,7 @@
cseq = (get_cseq(msg))->number;
} else {
/* use the same as in request */
- cseq = dlg->cseq[DLG_CALLER_LEG];
+ cseq = dlg->cseq[DLG_CALLEE_LEG];
}
avoid memory leak:
--- a/modules/dialog/dlg_hash.c Fri Jun 14 13:40:12 2013 +0200
+++ b/modules/dialog/dlg_hash.c Fri Jun 14 13:45:21 2013 +0200
@@ -485,7 +485,14 @@
char *p;
dlg->tag[leg].s = (char*)shm_malloc( tag->len + rr->len +
contact->len );
- dlg->cseq[leg].s = (char*)shm_malloc( cseq->len );
+ if(dlg->cseq[leg].s){
+ if (dlg->cseq[leg].len < cseq->len) {
+ shm_free(dlg->cseq[leg].s);
+ dlg->cseq[leg].s = (char*)shm_malloc(cseq->len);
+ }
+ }else{
+ dlg->cseq[leg].s = (char*)shm_malloc( cseq->len );
+ }
if ( dlg->tag[leg].s==NULL || dlg->cseq[leg].s==NULL) {
LM_ERR("no more shm mem\n");
if (dlg->tag[leg].s)
_______________________________________________
SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list
sr-users(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
4168
days inactive
4185
days old
5 comments
3 participants
participants (3)
-
Daniel-Constantin Mierla -
Halina Nowak -
Randhir Singh