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

Fix failure to start operational advertising #6647

Closed

Conversation

jmartinez-silabs
Copy link
Member

@jmartinez-silabs jmartinez-silabs commented May 10, 2021

Problem

Starting an Advertise operational node fails with
[SVR] Failed to start operational advertising: CHIP Error 4000030 (0x003D091E): Invalid string length

SrpClient::kMaxInstanceNameSize = 33 ([Node]-[Fabric] ID in hex - 16+1+16)
Srp client service mInstanceName is declared char mInstanceName[kMaxInstanceNameSize]; so it is missing a space for the NULL Character.

_AddSrpService or _RemoveSrpService fails on
VerifyOrExit(strlen(aInstanceName) < SrpClient::kMaxInstanceNameSize, error = CHIP_ERROR_INVALID_STRING_LENGTH); as it verifies < 33

Summary of Changes

Add +1 for Null character. Without the Null character the srp server would fail to parse the service.
With this VerifyOrExit(strlen(aInstanceName) < SrpClient::kMaxInstanceNameSize, error = CHIP_ERROR_INVALID_STRING_LENGTH); now accepts the full srp instance name ([Node]-[Fabric] ID in hex - 16+1+16)

@jmartinez-silabs jmartinez-silabs changed the title Fix failurre tu start operation advertising Fix failure to start operation advertising May 10, 2021
@jmartinez-silabs jmartinez-silabs changed the title Fix failure to start operation advertising Fix failure to start operational advertising May 10, 2021
@jmartinez-silabs
Copy link
Member Author

Just saw this PR #6626
to address the same issue. I think the other values modified in that PR were ok.
I can close this PR if the one gets merge

@jmartinez-silabs
Copy link
Member Author

Closing in favor of #6626

@jmartinez-silabs jmartinez-silabs deleted the srp_strlen branch May 11, 2021 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants