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

[dns-sd] fix SRP service name length #6626

Merged
merged 2 commits into from
May 13, 2021

Conversation

wgtdkp
Copy link
Contributor

@wgtdkp wgtdkp commented May 10, 2021

Problem

The SRP service name buffers are too small.

Summary of Changes

Fix SRP service & host name buffer issue.

@andy31415
Copy link
Contributor

On this PR I see the 'run unit tests' stuck in the run state (without output unfortunately) for 50 minutes. Unsure if PR-specific or a flake. Given that every one of the runners does this, it may be something in the change itself.

@andy31415
Copy link
Contributor

I cancelled the workflows with the intent of restarting them, however I do not seem to get a 'restart all' button yet.
If I am unable to do so, @wgtdkp could you push a minor update to kick the presubmits again?

@github-actions
Copy link

Size increase report for "esp32-example-build" from 8185019

File Section File VM
chip-all-clusters-app.elf .flash.rodata -8 -8
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
[Unmapped],0,8
.flash.rodata,-8,-8


@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from 8185019

File Section File VM
chip-lighting.elf rodata 56 52
chip-lighting.elf text 40 40
chip-lighting.elf device_handles -8 -8
chip-lighting.elf bss 0 -13
chip-lighting.elf [LOAD #3 [RW]] 0 -19
chip-shell.elf [LOAD #3 [RW]] 0 31
chip-shell.elf bss 0 -31
chip-lock.elf rodata 56 52
chip-lock.elf text 36 36
chip-lock.elf device_handles -4 -4
chip-lock.elf bss 0 -9
chip-lock.elf [LOAD #3 [RW]] 0 -23
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-lighting.elf and ./pull_artifact/chip-lighting.elf:

sections,vmsize,filesize
.debug_info,0,1497
rodata,52,56
text,40,40
.symtab,0,32
.debug_str,0,28
.debug_frame,0,12
.debug_line,0,3
.debug_loc,0,-4
device_handles,-8,-8
bss,-13,0
[LOAD #3 [RW]],-19,0

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_info,0,481
[LOAD #3 [RW]],31,0
.debug_str,0,28
.debug_frame,0,12
.debug_loc,0,4
.debug_line,0,3
bss,-31,0

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.debug_info,0,777
rodata,52,56
text,36,36
.symtab,0,32
.debug_str,0,28
.debug_frame,0,12
.debug_line,0,3
.debug_loc,0,-4
device_handles,-4,-4
bss,-9,0
[LOAD #3 [RW]],-23,0


@woody-apple
Copy link
Contributor

I kicked the Tests again, for some reason they're getting stuck...

@andy31415
Copy link
Contributor

I kicked the Tests again, for some reason they're getting stuck...

I am totally puzzled here - I have never seen these tests hang on any other PR, however I am unclear on why this PR would cause system tests to misbehave either (and across all compilers to boot). I did copy the PR and tried locally, it passes (except I found a Mdns hang which I fixed in #6721). Kangping reported his workstation hangs on TestSystemObject but I could not see that.

@woody-apple woody-apple merged commit 9d0613e into project-chip:master May 13, 2021
@wgtdkp
Copy link
Contributor Author

wgtdkp commented May 13, 2021

@woody-apple @andy31415 Thanks for helping resolve the test hang issue. I am sorry that it is too later for me to fix this issue last night, but it seems the hang issue is caused by the failure of the TestMdns test case and it is my change in this PR that is failing the TestMdns case :(

I submitted #6771 to fix the issue.

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.

9 participants