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 out of bound OLED font access #8145

Merged
merged 2 commits into from
Feb 11, 2020
Merged

Fix out of bound OLED font access #8145

merged 2 commits into from
Feb 11, 2020

Conversation

kitlaan
Copy link
Contributor

@kitlaan kitlaan commented Feb 11, 2020

Description

The default font is 1344 bytes, or a total of 224 glyphs (each 6-bytes wide).
OLED_FONT_END defaults to 224, which if used will then index off the end of
the font array. So either the documentation or code is wrong.

Instead of figuring out the rewording of the documentation, just change
the OLED_FONT_END default value to 223, to match the documentation and code.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@kitlaan
Fix out of bound OLED font access
c34da8e 
The default font is 1344 bytes, or a total of 224 glyphs (each 6-bytes wide).
OLED_FONT_END defaults to 224, which if used will then index off the end of
the font array. So either the documentation or code is wrong.

Instead of figuring out the rewording of the documentation, just change
the OLED_FONT_END default value to 223, to match the documentation and code.
@kitlaan
Copy link
Contributor Author

kitlaan commented Feb 11, 2020

As an aside... the default oled font has a bunch of trailing empty glyphs (25 of them). So both the font array and define could be reduced even further if someone wants some flash savings (possibly without any breaking changes). The array would then be 1200 bytes in length, and font would have last char of 199... if I maths properly.

@kitlaan
Add static assert to check array size
cca8496 
Build bomb if the font array size doesn't match to the defines.
@fauxpark fauxpark requested a review from a team February 11, 2020 05:00
@drashna drashna requested a review from a team February 11, 2020 18:11
@drashna
Copy link
Member

drashna commented Feb 11, 2020

@XScorpion2 Comments, thoughts, etc?

@XScorpion2
Copy link
Contributor

Ya, this is an appropriate change. I implemented end originally like a length value, but used it as last index value instead.

@zvecr zvecr merged commit 9456832 into qmk:master Feb 11, 2020
@zvecr
Copy link
Member

zvecr commented Feb 11, 2020

Thanks all!

@kitlaan kitlaan deleted the fix_oled_font branch February 12, 2020 00:09
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request Feb 12, 2020
@kitlaan @drashna
Fix out of bound OLED font access (qmk#8145)
d2a935a 
* Fix out of bound OLED font access

The default font is 1344 bytes, or a total of 224 glyphs (each 6-bytes wide).
OLED_FONT_END defaults to 224, which if used will then index off the end of
the font array. So either the documentation or code is wrong.

Instead of figuring out the rewording of the documentation, just change
the OLED_FONT_END default value to 223, to match the documentation and code.

* Add static assert to check array size

Build bomb if the font array size doesn't match to the defines.
akatrevorjay added a commit to akatrevorjay/qmk_firmware that referenced this pull request Feb 12, 2020
@akatrevorjay
Merge remote-tracking branch 'upstream/master'
419f13e 
* upstream/master:
  Align split_common/matrix.c with matrix.c (qmk#8153)
  format code according to conventions [skip ci]
  Align VUSB HID descriptors with LUFA/ChibiOS (qmk#7675)
  VIA support for the Think6.5 (qmk#8118)
  VIA support for Graystudio Space65 (qmk#8126)
  Fix out of bound OLED font access (qmk#8145)
fdidron pushed a commit to zsa/qmk_firmware that referenced this pull request Feb 14, 2020
@kitlaan @fdidron
Fix out of bound OLED font access (qmk#8145)
77e997e 
* Fix out of bound OLED font access

The default font is 1344 bytes, or a total of 224 glyphs (each 6-bytes wide).
OLED_FONT_END defaults to 224, which if used will then index off the end of
the font array. So either the documentation or code is wrong.

Instead of figuring out the rewording of the documentation, just change
the OLED_FONT_END default value to 223, to match the documentation and code.

* Add static assert to check array size

Build bomb if the font array size doesn't match to the defines.
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Feb 21, 2020
@kitlaan @HokieGeek
Fix out of bound OLED font access (qmk#8145)
35ad13a 
* Fix out of bound OLED font access

The default font is 1344 bytes, or a total of 224 glyphs (each 6-bytes wide).
OLED_FONT_END defaults to 224, which if used will then index off the end of
the font array. So either the documentation or code is wrong.

Instead of figuring out the rewording of the documentation, just change
the OLED_FONT_END default value to 223, to match the documentation and code.

* Add static assert to check array size

Build bomb if the font array size doesn't match to the defines.
fdidron pushed a commit to zsa/qmk_firmware that referenced this pull request Feb 26, 2020
@kitlaan @fdidron
Fix out of bound OLED font access (qmk#8145)
7938684 
* Fix out of bound OLED font access

The default font is 1344 bytes, or a total of 224 glyphs (each 6-bytes wide).
OLED_FONT_END defaults to 224, which if used will then index off the end of
the font array. So either the documentation or code is wrong.

Instead of figuring out the rewording of the documentation, just change
the OLED_FONT_END default value to 223, to match the documentation and code.

* Add static assert to check array size

Build bomb if the font array size doesn't match to the defines.
c0psrul3 pushed a commit to c0psrul3/qmk_firmware that referenced this pull request Mar 23, 2020
@kitlaan @c0psrul3
Fix out of bound OLED font access (qmk#8145)
12f5117 
* Fix out of bound OLED font access

The default font is 1344 bytes, or a total of 224 glyphs (each 6-bytes wide).
OLED_FONT_END defaults to 224, which if used will then index off the end of
the font array. So either the documentation or code is wrong.

Instead of figuring out the rewording of the documentation, just change
the OLED_FONT_END default value to 223, to match the documentation and code.

* Add static assert to check array size

Build bomb if the font array size doesn't match to the defines.
kylekuj pushed a commit to kylekuj/qmk_firmware that referenced this pull request Apr 21, 2020
@kitlaan @kujawakk
Fix out of bound OLED font access (qmk#8145)
fe623d4 
* Fix out of bound OLED font access

The default font is 1344 bytes, or a total of 224 glyphs (each 6-bytes wide).
OLED_FONT_END defaults to 224, which if used will then index off the end of
the font array. So either the documentation or code is wrong.

Instead of figuring out the rewording of the documentation, just change
the OLED_FONT_END default value to 223, to match the documentation and code.

* Add static assert to check array size

Build bomb if the font array size doesn't match to the defines.
jakeisnt pushed a commit to jakeisnt/qmk_firmware that referenced this pull request Aug 20, 2020
@kitlaan @jakeisnt
Fix out of bound OLED font access (qmk#8145)
a072cd3 
* Fix out of bound OLED font access

The default font is 1344 bytes, or a total of 224 glyphs (each 6-bytes wide).
OLED_FONT_END defaults to 224, which if used will then index off the end of
the font array. So either the documentation or code is wrong.

Instead of figuring out the rewording of the documentation, just change
the OLED_FONT_END default value to 223, to match the documentation and code.

* Add static assert to check array size

Build bomb if the font array size doesn't match to the defines.
@zigotica zigotica mentioned this pull request Sep 8, 2020
Closed
3 tasks
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
@kitlaan
Fix out of bound OLED font access (qmk#8145)
ffc1b78 
* Fix out of bound OLED font access

The default font is 1344 bytes, or a total of 224 glyphs (each 6-bytes wide).
OLED_FONT_END defaults to 224, which if used will then index off the end of
the font array. So either the documentation or code is wrong.

Instead of figuring out the rewording of the documentation, just change
the OLED_FONT_END default value to 223, to match the documentation and code.

* Add static assert to check array size

Build bomb if the font array size doesn't match to the defines.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XScorpion2 XScorpion2 XScorpion2 approved these changes

@fauxpark fauxpark fauxpark approved these changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
bug core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kitlaan @drashna @XScorpion2 @zvecr @fauxpark