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.

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

sip: fix TCP source port #921

Merged
merged 2 commits into from
Aug 30, 2023
Merged

Conversation

cspiel1
Copy link
Collaborator

@cspiel1 cspiel1 commented Aug 29, 2023

The TCP source has at the struct sip has to be set before the TCP connection
is opened. This commit fixes an issue that was introduced with
5c709fc which postponed the call to the
send_handler() after the TCP connection is established.

src/sip/request.c Outdated Show resolved Hide resolved
src/sip/dialog.c Outdated Show resolved Hide resolved
The TCP source has at the `struct sip` has to be set before the TCP connection
is opened. This commit fixes an issue that was introduced with
5c709fc which postponed the call to the
`send_handler()` after the TCP connection is established.
@@ -618,10 +611,13 @@ int sipreg_set_fbregint(struct sipreg *reg, uint32_t fbregint)
*/
void sipreg_set_srcport(struct sipreg *reg, uint16_t srcport)
{
if (!reg)
if (!reg || !reg->dlg)
Copy link
Member

Choose a reason for hiding this comment

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

Is setting the srcport only relevant for dlg? Or is there a use-case without it too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently not. We could add a setter void sip_request_set_srcport(...).

For now I couldn't find any call to plain sip_request() in re or baresip. And no call to sip_request_alloc() followed by sip_request_send().

For sipreg I think this is a correct condition, because it always has a dialog.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just saw that sip_request_alloc() is not in the API. So a setter sip_request_set_srcport() doesn't make sense.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe a bit misleading, I think the check !reg->dlg is not needed since it's not relevant for setting reg->srcport = srcport. As a caller of public sipreg_set_srcport I assume that srcport is set if I provide a valid sipreg object.

sip_dialog_set_srcport has a extra check for valid dlg, so this should fine.

src/sipreg/reg.c Outdated Show resolved Hide resolved
@sreimers sreimers merged commit 9ac5d49 into baresip:main Aug 30, 2023
29 checks passed
@cspiel1 cspiel1 deleted the main_sip_tcp_srcport branch August 31, 2023 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants