-
Notifications
You must be signed in to change notification settings - Fork 774
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix coverity warning #4003
Fix coverity warning #4003
Conversation
pjsip/src/pjsip-ua/sip_100rel.c
Outdated
if (rseq < 1) { | ||
PJ_LOG(4, (dd->inv->dlg->obj_name, | ||
"Ignoring 100rel response RSeq header value less than 1")); | ||
return PJSIP_EINVALIDMSG; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually as receiver, we better be lenient, especially if it is not so urgent/risky, perhaps printing warning is sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the message received with RSeq 0, currently it will be ignored in https://github.com/pjsip/pjproject/blob/coverity02/pjsip/src/pjsip-ua/sip_100rel.c#L303
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so should we return PJ_EIGNORED here, so it's the same as the old behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the message received with RSeq 0, currently it will be ignored in https://github.com/pjsip/pjproject/blob/coverity02/pjsip/src/pjsip-ua/sip_100rel.c#L303
The pointed code does not seem to check for rseq==0
, it checks for retransmission (where the original rseq may still be 0)? So IMO, perhaps better check the behavior when rseq ==0
(e.g: using SIPp) to at least avoid behavior change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rseq is unsigned, so the condition will always be true when rseq ==0
regardless the original rseq. If we want to avoid behavior change we should return PJ_EIGNORED (as @sauwming suggested) or just print the warning message.
Warnings from coverity:
From ref, the RSeq value must be set to 1 to 2**32 -1.
The patch will check for the RSeq value and make sure it is greater than 1.
Fix by adding check to the return value from
pjmedia_vid_codec_mgr_get_default_param()
.It should be client sock.