Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Moved 180 Ringing response from core to app (menu module) #1167

Closed
wants to merge 2 commits into from
Closed

Moved 180 Ringing response from core to app (menu module) #1167

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4810e6b
Allow 100 response code in sipsess_accept
juha-h Jul 24, 2024
51b0651
Allow skiping of reply by sipsess_accept
juha-h Jul 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/sipsess/accept.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ static void cancel_handler(void *arg)
* @param sessp Pointer to allocated SIP Session
* @param sock SIP Session socket
* @param msg Incoming SIP message
* @param scode Response status code
* @param scode Response status code or zero to skip sending of reply
* @param reason Response reason phrase
* @param rel100 Sending 1xx reliably supported, required or disabled
* @param cuser Contact username or URI
Expand Down Expand Up @@ -74,8 +74,8 @@ int sipsess_accept(struct sipsess **sessp, struct sipsess_sock *sock,
va_list ap;
int err;

if (!sessp || !sock || !msg || scode < 101 || scode > 299 ||
!cuser || !ctype)
if (!sessp || !sock || !msg || (scode != 0 && scode < 100) ||
scode > 299 || !cuser || !ctype)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure with this. Why not directly use sipsess_alloc()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll investigate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sipsess_alloc() is not in re API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cspiel1 What is wrong with sipsess_accept()? In addition to sipsess_alloc() it does lots other stuff that would also need to be handled separately:

err = sip_dialog_accept(&sess->dlg, msg);
	if (err)
		goto out;

	hash_append(sock->ht_sess,
		    hash_joaat_str(sip_dialog_callid(sess->dlg)),
		    &sess->he, sess);

	sess->msg = mem_ref((void *)msg);

	err = sip_strans_alloc(&sess->st, sess->sip, msg, cancel_handler,
			       sess);

So I would prefer to keep the PR as it currently is. I have made tests and they all have worked fine.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion sipsess_accept() should send a reply. @alfredh and @sreimers please review and decide!

Should we revert this, when we have another solution?

return EINVAL;

err = sipsess_alloc(&sess, sock, cuser, ctype, NULL, authh, aarg, aref,
Expand Down Expand Up @@ -104,7 +104,7 @@ int sipsess_accept(struct sipsess **sessp, struct sipsess_sock *sock,

va_start(ap, fmt);

if (scode > 100 && scode < 200) {
if (scode >= 100 && scode < 200) {
err = sipsess_reply_1xx(sess, msg, scode, reason, rel100, desc,
fmt, &ap);
}
Expand Down
Loading