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

Be more careful when adding delta #15

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

simoncozens
Copy link

@simoncozens simoncozens commented Nov 9, 2024

Rust has helpful convenience methods for avoiding overflows with arithmetic and avoiding casts; this prevents harfbuzz/rustybuzz#142.

@alerque
Copy link
Member

alerque commented Nov 9, 2024

As plausible as it seemed, this isn't quite the right fix: end should never have been usize in the first place. I can submit a PR when the dust settles (although I have some questions about why this fork has a separate codebase that can get out of sync and need this kind of PR instead of being a feature flag on the original port and/or being a PoC branch continuously rebased on it).

@behdad
Copy link
Member

behdad commented Nov 11, 2024

Yeah let's keep it close to HB. If overflow happens, that shows a bug in HB code. In this case the fix is indeed to use a signed int.

@behdad
Copy link
Member

behdad commented Nov 11, 2024

I have some questions about why this fork has a separate codebase that can get out of sync and need this kind of PR instead of being a feature flag on the original port and/or being a PoC branch continuously rebased on it

I don't think there's a good reason for it, except that the respective owners didn't reach an agreement:

googlefonts/fontations#956

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