-
Notifications
You must be signed in to change notification settings - Fork 75
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
reading/writing xproto.SetupRequest is missing padding #24
Comments
How would one know that AuthorizationProtocolName and AuthorizationProtocolData are to be padded, short of reading xlib code? Note that conn.go doesn't use SetupRequest to setup the connection (and that does pad the fields). |
I did this by trying to write my own version of the I then looked at the https://github.com/BurntSushi/xgb/blob/master/conn.go#L57 It doesn't use the xproto.SetupRequest stuff (as you mention), but instead re-implement it "by hand", and there was the missing padding! |
It'd be kind of nice if |
It's not that simple unfortunately. The |
Oh, I see... Many of those look like they're helper functions, but not all, getting the last reference out might be tricky. I'd still like to use |
As in, "we already copy/paste functions wholesale, it's all right"? Sure, but if you copy/pasted If it wasn't going through the compiler, I'd have just sent a pull request, but I'm not as comfortable making that change to the compiler, sorry... |
Yeah, that's exactly the problem: X11 padding rules aren't really clear (or documented at all). People will tell you that "it's just like C struct padding", except it isn't, not in any interesting place, at least. |
Might be a bug in the XML, then? Here, it's clearly mentioned: http://www.x.org/releases/current/doc/xproto/x11protocol.html#Encoding::Connection_Setup I hear you on the padding rules not being the clearest! |
(I'm referring to the "p" and "q" of the very first structure description there in the "Connection Setup" section) |
You may be onto something, I ran a capture myself and xcb proper seems to have similar problems. I'll inquire upstream. |
Fields AuthorizationProtocolName and AuthorizationProtocolData of SetupRequest should be padded: http://www.x.org/releases/current/doc/xproto/x11protocol.html#Encoding::Connection_Setup The problem was discovered by github user pphaneuf while trying to use xgb to write his own implementation of the connection handshake. Neither xgb nor xcb actually use code generated for SetupRequest for the handshake, which is why this bug went unnoticed. BurntSushi/xgb#24 Alessandro Arzilli. Reviewed-by: Christian Linhart <chris@demorecorder.com>
The
AuthorizationProtocolName
andAuthorizationProtocolData
fields are supposed to be padded to a multiple of 4, but it's not done:xgb/xproto/xproto.go
Line 5805 in eabb7fe
xgb/xproto/xproto.go
Line 5853 in eabb7fe
I see some mentions of
xgb.Pad()
at line 5830, but it comes out a bit short when actually using it?The text was updated successfully, but these errors were encountered: