Bug 65626 - cairo-script-interpreter API changes
Summary: cairo-script-interpreter API changes
Status: RESOLVED MOVED
Alias: None
Product: cairo
Classification: Unclassified
Component: general (show other bugs)
Version: 1.12.14
Hardware: All All
: medium normal
Assignee: Chris Wilson
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: cairo-1.14
  Show dependency treegraph
 
Reported: 2013-06-11 01:58 UTC by Daniel Macks
Modified: 2018-08-25 13:56 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Disable CSI by default and note the API is not stable (2.12 KB, patch)
2015-06-26 00:10 UTC, Bryce Harrington
Details | Splinter Review

Description Daniel Macks 2013-06-11 01:58:23 UTC
Going from cairo-1.12.8 to 1.12.10, cairo-script-interpreter.h was changed (via 146da77d85b304651949a819bc8b0a74819f0416) to add a new member to struct cairo_script_interpreter_hooks. As far as I can tell, callers create variables of this type themselves (via malloc or simple variable declarations) and access its members before passing it to cairo_script_interpreter_install_hooks() in libcairo. That means code that was compiled with the "old" typedef (that did not have the new member) would be passing a pointer to a chunk of memory that does not include allocated storage for that new member. But then "new" libcairo tries to read the new member's value, meaning it's accessing memory that isn't intended to be allocated to that struct. I don't know much about libcairo's internals, but I don't see any protection or struct versioning that would prevent this from happening, with undefined results.
Comment 1 Chris Wilson 2013-06-11 08:24:43 UTC
Sure, I have never considered it to be more than a private library for the internal analysis tools. It needs a lot more refinement for it to be suitable even for the few use cases I have in mind.
Comment 2 Daniel Macks 2013-07-01 07:10:10 UTC
Makes sense. Please announce this loudly and in many places...Apple's xquartz and the Debian linux distro both include it as a public shared library with public headers. Maybe make it not default to "enabled" in ./configure until it at least has a stable interface?
Comment 3 Bryce Harrington 2015-06-26 00:10:32 UTC
Created attachment 116723 [details] [review]
Disable CSI by default and note the API is not stable

This patch adds some warnings that CSI is not stable and disables it in configure.ac by default as suggested.

To be honest I'm not really sure this is the right thing to do here.  Since it's been a while since CSI's interface changed it seems like there'd be little harm in just versioning it as 0.1, labelling it unstable, and leave it enabled by default.  But I'm not sure what the right way to version it would be...

What do others think?
Comment 4 Chris Wilson 2015-06-26 07:23:17 UTC
You want to always compile it, but exclude it from the installation target (by default at least).
Comment 5 GitLab Migration User 2018-08-25 13:56:27 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/cairo/cairo/issues/279.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.