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(arm): grow slice not clear mem #674

Merged
merged 1 commit into from
Jul 17, 2024
Merged

fix(arm): grow slice not clear mem #674

merged 1 commit into from
Jul 17, 2024

Conversation

liuq19
Copy link
Collaborator

@liuq19 liuq19 commented Jul 16, 2024

No description provided.

new := GrowSlice(et, *old, newLen)

// we sould clear the memory from [oldLen:newLen]
if et.PtrData == 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

为啥noscan需要清理?scan类型不需要清理??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

是的,因为runtime.growslice 对于 [oldlen, newlen) 这段区间的的非指针数据是不需要清空的

Copy link
Collaborator

Choose a reason for hiding this comment

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

那noscan数据就脏着呗,不清理有啥问题吗

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

因为 makeslice 里面长度会变成 newLen, 而且如果遇到非匹配类型,会跳过这些类型。因此需要清空

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

因为makeslice 的语义其实是需要初始化所有元素的,因为如果decode 遇到不需要解析的类型会直接skip,不初始化可能会有脏数据

internal/rt/growslice.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jul 17, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 47.82609% with 12 lines in your changes missing coverage. Please review.

Project coverage is 52.63%. Comparing base (125ff79) to head (d347689).

Files Patch % Lines
internal/rt/stubs.go 47.36% 7 Missing and 3 partials ⚠️
internal/rt/growslice.go 50.00% 1 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #674      +/-   ##
==========================================
- Coverage   52.85%   52.63%   -0.23%     
==========================================
  Files         174      142      -32     
  Lines       11470    11397      -73     
==========================================
- Hits         6063     5999      -64     
+ Misses       5071     5068       -3     
+ Partials      336      330       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@liuq19 liuq19 merged commit 65b7fbe into main Jul 17, 2024
43 checks passed
@liuq19 liuq19 deleted the fix/go121 branch July 17, 2024 04:50
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.

3 participants