-
Notifications
You must be signed in to change notification settings - Fork 606
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
[var] Fix hb_ot_var_get_axis_infos's offset semantic #2221
Conversation
start_offset = hb_min (start_offset, count); | ||
|
||
count -= start_offset; | ||
axes_array += start_offset; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the part that was wrong, start_offset isn't intended to be added to axes_array index but just used just as a AxisRecord index offset.
g_assert (info.tag == HB_TAG ('w','g','h','t')); | ||
|
||
hb_ot_var_get_axis_infos (face, 1, &c, &info); | ||
g_assert (info.tag == HB_TAG ('w','d','t','h')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This didn't work before this change. was hammering stack…
@khaledhosny @behdad What do you suggest? Do you know clients that were using offset of the API? This overflows buffer for clients who iterating axis in usual way of HarfBuzz API. |
9c86877
to
e54c6be
Compare
Pango fortunately doesn't use its offset, guess no client would use as it is broken anyway. |
e54c6be
to
982ad40
Compare
The API was adding offset to input's infos buffer index also which is unusual between our APIs and wrong.
982ad40
to
333831c
Compare
Any pointers? |
Yeah I think is safe to fix. No idea why I didn't just use sub_array to begin with... But yeah I put TODOs where I insert bugs apparently! |
Thank you :) |
The API was adding offset to input's infos buffer index also which is
unusual between our APIs and wrong.