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

fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() #44149

Merged
merged 1 commit into from
Oct 9, 2024
Merged

fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() #44149

merged 1 commit into from
Oct 9, 2024

Conversation

ckerr
Copy link
Member

@ckerr ckerr commented Oct 8, 2024 •
edited
Loading

Description of Change

Fixup to #43803 / e894839 that actually fixes the warning.

Takeaway lesson: give the compiler enough information to know that the array indices are always in the array's valid range.

Warnings fixed by this PR:

2024-10-04T05:18:49.7736915Z ../../electron/shell/browser/ui/inspectable_web_contents.cc:132:46: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage]
2024-10-04T05:18:49.7737765Z   132 |       return blink::ZoomFactorToZoomLevel(*--iter);
2024-10-04T05:18:49.7738163Z       |                                              ^~~~
2024-10-04T05:18:49.7738801Z ../../electron/shell/browser/ui/inspectable_web_contents.cc:132:46: note: See //docs/unsafe_buffers.md for help.
2024-10-04T05:18:49.7739875Z ../../electron/shell/browser/ui/inspectable_web_contents.cc:133:19: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage]
2024-10-04T05:18:49.7740573Z   133 |     if (!out && ++iter != kEnd)
2024-10-04T05:18:49.7740875Z       |                   ^~~~
2024-10-04T05:18:49.7741546Z ../../electron/shell/browser/ui/inspectable_web_contents.cc:133:19: note: See //docs/unsafe_buffers.md for help.
2024-10-04T05:18:49.7742134Z 2 errors generated.

Checklist

Release Notes

Notes: none

@ckerr ckerr added semver/patch backwards-compatible bug fixes target/31-x-y PR should also be added to the "31-x-y" branch. target/32-x-y PR should also be added to the "32-x-y" branch. target/33-x-y PR should also be added to the "33-x-y" branch. labels Oct 8, 2024
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Oct 8, 2024
@ckerr ckerr force-pushed the fix/fixup-43803-Wunsafe-buffer-usage-in-GetNextZoomLevel() branch from ba8dee4 to 1b298e0 Compare October 8, 2024 15:04
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Oct 9, 2024
@ckerr ckerr merged commit b5227b4 into main Oct 9, 2024
54 checks passed
@release-clerk Release Clerk
Copy link

release-clerk bot commented Oct 9, 2024

No Release Notes

@ckerr ckerr deleted the fix/fixup-43803-Wunsafe-buffer-usage-in-GetNextZoomLevel() branch October 9, 2024 14:12
@trop
Copy link
Contributor

trop bot commented Oct 9, 2024

I was unable to backport this PR to "31-x-y" cleanly;
you will need to perform this backport manually.

@trop trop bot added needs-manual-bp/31-x-y and removed target/31-x-y PR should also be added to the "31-x-y" branch. labels Oct 9, 2024
This was referenced Oct 9, 2024
Merged
Merged
@trop
Copy link
Contributor

trop bot commented Oct 9, 2024

I have automatically backported this PR to "33-x-y", please check out #44163

@trop trop bot added in-flight/33-x-y and removed target/33-x-y PR should also be added to the "33-x-y" branch. labels Oct 9, 2024
@trop
Copy link
Contributor

trop bot commented Oct 9, 2024

I have automatically backported this PR to "32-x-y", please check out #44164

@trop trop bot added in-flight/32-x-y merged/32-x-y PR was merged to the "32-x-y" branch. merged/33-x-y PR was merged to the "33-x-y" branch. and removed target/32-x-y PR should also be added to the "32-x-y" branch. in-flight/32-x-y in-flight/33-x-y labels Oct 9, 2024
ckerr added a commit that referenced this pull request Oct 12, 2024
@ckerr
fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() (#44149)
32660d1 
fixup e894839 really fix the warning this time
@ckerr ckerr mentioned this pull request Oct 12, 2024
Merged
@trop
Copy link
Contributor

trop bot commented Oct 12, 2024

@ckerr has manually backported this PR to "31-x-y", please check out #44216

codebytere pushed a commit that referenced this pull request Oct 13, 2024
@ckerr
fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() (#44149) (#…
dce705e 
…44216)

fixup e894839 really fix the warning this time
@trop trop bot added merged/31-x-y PR was merged to the "31-x-y" branch. and removed in-flight/31-x-y labels Oct 13, 2024
yangannyx pushed a commit to yangannyx/electron that referenced this pull request Oct 21, 2024
@ckerr @yangannyx
fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() (electron#4…
7a2994c 
…4149)

fixup e894839 really fix the warning this time
yangannyx pushed a commit to yangannyx/electron that referenced this pull request Oct 21, 2024
@ckerr @yangannyx
fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() (electron#4…
2afeede 
…4149)

fixup e894839 really fix the warning this time
yangannyx pushed a commit to yangannyx/electron that referenced this pull request Oct 21, 2024
@ckerr @yangannyx
fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() (electron#4…
a35c12c 
…4149)

fixup e894839 really fix the warning this time
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mlaurencin mlaurencin mlaurencin approved these changes

@jkleinsc jkleinsc jkleinsc approved these changes

Assignees
No one assigned
Labels
merged/31-x-y PR was merged to the "31-x-y" branch. merged/32-x-y PR was merged to the "32-x-y" branch. merged/33-x-y PR was merged to the "33-x-y" branch. semver/patch backwards-compatible bug fixes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ckerr @jkleinsc @mlaurencin