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() #43803

Merged
merged 1 commit into from
Sep 21, 2024
Merged

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

merged 1 commit into from
Sep 21, 2024

Conversation

ckerr
Copy link
Member

@ckerr ckerr commented Sep 19, 2024 •
edited
Loading

Description of Change

Part 6 in a series to fix -Wunsafe-buffer-usage warnings in electron shell/.

This PR fixes three warnings in GetNextZoomLevel(). The previous code is reasonable IMO but still kicks a clang warning because of its use of pointer math. 🤷‍♂️ This PR uses iterators and begin / end / find_if instead.

The warnings fixed by this PR:

../../electron/shell/browser/ui/inspectable_web_contents.cc:128:33: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage]
  128 |     if (!blink::ZoomValuesEqual(kPresetZoomFactors[i], factor))
      |                                 ^~~~~~~~~~~~~~~~~~
../../electron/shell/browser/ui/inspectable_web_contents.cc:128:33: note: pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions
../../electron/shell/browser/ui/inspectable_web_contents.cc:131:43: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage]
  131 |       return blink::ZoomFactorToZoomLevel(kPresetZoomFactors[i - 1]);
      |                                           ^~~~~~~~~~~~~~~~~~
../../electron/shell/browser/ui/inspectable_web_contents.cc:131:43: note: pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions
../../electron/shell/browser/ui/inspectable_web_contents.cc:133:43: error: unsafe buffer access [-Werror,-Wunsafe-buffer-usage]
  133 |       return blink::ZoomFactorToZoomLevel(kPresetZoomFactors[i + 1]);
      |                                           ^~~~~~~~~~~~~~~~~~
../../electron/shell/browser/ui/inspectable_web_contents.cc:133:43: note: pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions

Checklist

Release Notes

Notes: none.

@ckerr ckerr added semver/patch backwards-compatible bug fixes target/30-x-y PR should also be added to the "30-x-y" branch. 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 Sep 19, 2024
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Sep 19, 2024
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Sep 20, 2024
@ckerr ckerr merged commit e894839 into main Sep 21, 2024
69 checks passed
@ckerr ckerr deleted the fix/Wunsafe-buffer-usage-warning-GetNextZoomLevel branch September 21, 2024 22:21
@release-clerk Release Clerk
Copy link

release-clerk bot commented Sep 21, 2024

No Release Notes

@trop
Copy link
Contributor

trop bot commented Sep 21, 2024

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

@trop trop bot removed the target/30-x-y PR should also be added to the "30-x-y" branch. label Sep 21, 2024
@trop
Copy link
Contributor

trop bot commented Sep 21, 2024

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

@trop
Copy link
Contributor

trop bot commented Sep 21, 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/32-x-y needs-manual-bp/31-x-y and removed target/32-x-y PR should also be added to the "32-x-y" branch. target/31-x-y PR should also be added to the "31-x-y" branch. labels Sep 21, 2024
@trop trop bot mentioned this pull request Sep 21, 2024
Merged
@trop
Copy link
Contributor

trop bot commented Sep 21, 2024

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

@trop trop bot added in-flight/33-x-y merged/33-x-y PR was merged to the "33-x-y" branch. and removed target/33-x-y PR should also be added to the "33-x-y" branch. in-flight/33-x-y labels Sep 21, 2024
ckerr added a commit that referenced this pull request Sep 23, 2024
@ckerr
fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() (#43803)
80d5b53
@ckerr ckerr mentioned this pull request Sep 23, 2024
Merged
@trop
Copy link
Contributor

trop bot commented Sep 23, 2024

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

MarshallOfSound pushed a commit that referenced this pull request Sep 24, 2024
@ckerr
fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() (#43894)
4e74793 
fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() (#43803)
@trop trop bot added merged/32-x-y PR was merged to the "32-x-y" branch. and removed in-flight/32-x-y labels Sep 24, 2024
ckerr added a commit that referenced this pull request Sep 24, 2024
@ckerr
fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() (#43803)
4a0c5e4
@ckerr ckerr mentioned this pull request Sep 24, 2024
Merged
@trop
Copy link
Contributor

trop bot commented Sep 24, 2024

@ckerr has manually backported this PR to "main", please check out #43926

@trop trop bot added the in-flight/main label Sep 24, 2024
@trop
Copy link
Contributor

trop bot commented Sep 24, 2024

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

codebytere pushed a commit that referenced this pull request Sep 25, 2024
@ckerr
fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() (31-x-y) (#…
f15fea8 
…43926)

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

* empty commit
@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 Sep 25, 2024
trop bot added a commit that referenced this pull request Sep 25, 2024
@trop @ckerr
fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() (#43803)
4c12075 
Co-authored-by: Charles Kerr <[email protected]>
@trop trop bot mentioned this pull request Sep 25, 2024
Merged
ckerr added a commit that referenced this pull request Sep 25, 2024
@trop @ckerr
fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() (#43941)
9829cb5 
fix: -Wunsafe-buffer-usage warnings in GetNextZoomLevel() (#43803)

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Charles Kerr <[email protected]>
@trop trop bot added merged/30-x-y PR was merged to the "30-x-y" branch. and removed in-flight/30-x-y needs-manual-bp/30-x-y labels Sep 25, 2024
@ckerr ckerr mentioned this pull request Oct 8, 2024
Merged
3 tasks
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…
55dc4b6 
…3803)
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…
b9d0bb9 
…3803)
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…
50a808c 
…3803)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarshallOfSound MarshallOfSound MarshallOfSound approved these changes

Assignees
No one assigned
Labels
in-flight/main merged/30-x-y PR was merged to the "30-x-y" branch. 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.
2 participants
@ckerr @MarshallOfSound