Bug 65626

Summary:	cairo-script-interpreter API changes
Product:	cairo	Reporter:	Daniel Macks <dmacks>
Component:	general	Assignee:	Chris Wilson <chris>
Status:	RESOLVED MOVED	QA Contact:	cairo-bugs mailing list <cairo-bugs>
Severity:	normal
Priority:	medium
Version:	1.12.14
Hardware:	All
OS:	All
Whiteboard:
i915 platform:		i915 features:
Bug Depends on:
Bug Blocks:	68382
Attachments:	Disable CSI by default and note the API is not stable

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.