Bug 14511 - XDamageQueryVersion/XCompositeQueryVersion return larger-than-requested versions
Summary: XDamageQueryVersion/XCompositeQueryVersion return larger-than-requested versions
Status: RESOLVED INVALID
Alias: None
Product: xorg
Classification: Unclassified
Component: Lib/other (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Linux (All)
: medium minor
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: xorg-7.4
  Show dependency treegraph
 
Reported: 2008-02-15 03:26 UTC by Nathaniel Smith
Modified: 2008-05-19 02:30 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nathaniel Smith 2008-02-15 03:26:07 UTC
Running against Debian's xvfb ("2:1.4.1~git20080105-1"), XDamageQueryVersion seems to return versions higher than requested.  Essentially:

int major, minor;
major = 1;
minor = 0;
XDamageQueryVersion(display, &major, &minor);

---> now major = 1, minor = 1.

This contradicts the protocol definition:
	"The client sends the highest supported version to the server and
	the server sends the highest version it supports, but **no higher than
	the requested version**."

Obviously, this is easy to work around, since we know 1.1 is backwards compatible with 1.0, but it still seems to be a bug in either the implementation or the spec.
Comment 1 Nathaniel Smith 2008-02-15 03:29:43 UTC
XCompositeQueryVersion has exactly the same bug -- the same behavior and the same documentation.
Comment 2 Julien Cristau 2008-02-17 07:59:26 UTC
The server actually compares the version it supports and the version provided by the client.  libXdamage however uses the version from the <X11/extensions/damagewire.h> header it was built against.  I don't know if this is really a bug, or intentional behaviour.
Comment 3 Nathaniel Smith 2008-02-18 00:58:39 UTC
Do you mean that libXdamage is always returning the version compiled into it, regardless of version that the server claims to support?  If so, that is a very serious bug!  It's entirely plausible that a new libXdamage will be used to connect to an old server.
Comment 4 Julien Cristau 2008-02-18 04:57:18 UTC
(In reply to comment #3)
> Do you mean that libXdamage is always returning the version compiled into it,
> regardless of version that the server claims to support?  If so, that is a very
> serious bug!  It's entirely plausible that a new libXdamage will be used to
> connect to an old server.
> 
No, I mean that XDamageQueryVersion() sends a DamageQueryVersion request to the server using the version of the damage protocol headers present at build time.  The value returned to the user is then whatever the server replies.  So the values of *major_versionp and *minor_versionp passed to libXdamage don't matter.
Comment 5 Adam Jackson 2008-03-24 08:09:40 UTC
This is actually a problem for all of libX{damage,fixes,composite}.

I'm not really clear _why_ the code in these three is more complicated than just calling down to libXext.
Comment 6 Daniel Stone 2008-03-25 09:13:39 UTC
and it looks like it hits libXrandr too.
Comment 8 Aaron Plattner 2008-04-16 14:41:19 UTC
I don't have a man page for XDamageQueryVersion, but the one for XCompositeQueryVersion says this:

   XCompositeQueryVersion determines if the X Server supports a version of the X
   Composite Extension which is compatible with the client library. A non-zero
   Status is returned if a compatible version of the extension is supported,
   otherwise a zero Status is returned. If the extension is supported, the major
   and minor version numbers are returned to indicate the level of Composite
   Extension support.

In addition,

   major_versionp
       Pointer to integer where the major version of the Composite Extension supported
       by the X server will be stored.

   minor_versionp
       Pointer to integer where the minor version of the Composite Extension supported
       by the X server will be stored.

This implies to me that the client library should report to the server the version it was compiled with, and then should return (through its major and minor pointer arguments), the resulting version.  I don't think applications are supposed to pass data into the client libraries through the pointer arguments to the X*QueryVersion functions.
Comment 9 Nathaniel Smith 2008-04-16 19:12:51 UTC
I am more interested in what interface is appropriate to expose relevant functionality than I am in what the man page happens to say.

@Aaron: How would you expose the version-negotiation functionality in these extension protocols, to library users?  (All the other X extensions that I am aware of use the style of interface described in this bug, where the application writer feeds in the version number they expect.)
Comment 10 Aaron Plattner 2008-04-16 20:29:53 UTC
Which X extensions are these?  A quick skim of the libraries revealed none that treat the pointers as inputs.  In fact, some go so far as to explicitly name them "major_version_return" and "minor_version_return".

Returning the version number and letting the application decide whether or not it can handle it seems fine to me.  Moreover, it's too late to change the interface because applications today pass pointers to uninitialized memory, expecting it to be filled in by these functions.
Comment 11 Peter Hutterer 2008-05-19 02:30:13 UTC
(In reply to comment #9)
> I am more interested in what interface is appropriate to expose relevant
> functionality than I am in what the man page happens to say.
> 
> @Aaron: How would you expose the version-negotiation functionality in these
> extension protocols, to library users?  

you can't. Xlib always does a bit of magic to hide the pure protocol, and this is just one of the many things it does for you. If you want to expose this particular thing to the application, you will have to check for the return codes.

Anyway, man pages and function prototypes are updated to clarify this a bit better.

Pushed as 
libXcomposite: 16ae68423eb30639d880445c6bbe70d539e4b198
libXdamage: 0ca7d78aeb8035ef52c1415170e1257493d789fd
libXfixes: 839ef4a38dceec053c3fb33878e59eb26bd8d580
libXrandr: 332eee90c4d00be3b11049e0261323abe89a96dc
libXtst: 56bc832134b4f6884999797f0f0c1b846602088d
libXinerama: c5ac895a7dabe5a46e33e733771f20cc08e72d95
libXfontcache: dd8d4c3bf0025d7a79de623d8b6eac192a832e0f
libXRes: 168153d1e7d196ca46c5b2e286fcf7e7793f2804
libXScreensaver: ff9c27b08754c160256fe4d75cc4fbe07083f9d4


> (All the other X extensions that I am
> aware of use the style of interface described in this bug, where the
> application writer feeds in the version number they expect.)

uhm. no actually. I just looked through nearly all extensions and didn't find any that do so.


Changing bug to INVALID, as it is caused by a misunderstanding of the API.



Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct.