Skip to content
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

Specify XCB types more carefully to fix crashes on 32-bit systems #132

Merged
merged 1 commit into from
Mar 26, 2020

Conversation

mitya57
Copy link
Contributor

@mitya57 mitya57 commented Mar 23, 2020

@grulja
Copy link
Collaborator

grulja commented Mar 23, 2020

How do I verify these changes are correct? I didn't write this part of code and to be honest I don't even know what is this used for.

@MartinBriza do you know?

@mitya57
Copy link
Contributor Author

mitya57 commented Mar 23, 2020

This part is used for setting _GTK_THEME_VARIANT property on windows when dark theme is in use, to make the window manager draw dark decorations.

You can verify it by comparing the signatures here with real signatures in xcb/xcb.h and xcb/xproto.h. You can also try to run any application with QT_STYLE_OVERRIDE=Adwaita-Dark on a 32-bit system, it should not crash.

Note that adwaita-qt should be compiled without ADWAITA_HAVE_X11. Actually I haven't figured out how to compile it with that flag, for example it needs qtx11extras but CMake does not add needed flags for that library.

@grulja
Copy link
Collaborator

grulja commented Mar 25, 2020

I checked both header files, but I don't see these definitions.

@mitya57
Copy link
Contributor Author

mitya57 commented Mar 26, 2020

For me they are defined as follows:

/* In /usr/include/xcb/xcb.h */
xcb_connection_t *xcb_connect(const char *displayname, int *screenp);

/* In /usr/include/xcb/xproto.h */
xcb_intern_atom_cookie_t
xcb_intern_atom (xcb_connection_t *c,
                 uint8_t           only_if_exists,
                 uint16_t          name_len,
                 const char       *name);

xcb_intern_atom_reply_t *
xcb_intern_atom_reply (xcb_connection_t          *c,
                       xcb_intern_atom_cookie_t   cookie  /**< */,
                       xcb_generic_error_t      **e);

xcb_void_cookie_t
xcb_change_property (xcb_connection_t *c,
                     uint8_t           mode,
                     xcb_window_t      window,
                     xcb_atom_t        property,
                     xcb_atom_t        type,
                     uint8_t           format,
                     uint32_t          data_len,
                     const void       *data);

where the types are defined as:

/* In /usr/include/xcb/xcb.h */
typedef struct {
    unsigned int sequence;  /**< Sequence number */
} xcb_void_cookie_t;

/* In /usr/include/xcb/xproto.h */
typedef uint32_t xcb_window_t;
typedef uint32_t xcb_atom_t;

Note that any pointers may be safely replaced with void *, as that is the same from ABI / calling convention point of view.

@grulja
Copy link
Collaborator

grulja commented Mar 26, 2020

You are correct, I wasn't searching them under correct name. Looks good to me now.

@grulja grulja merged commit 1f3a43a into FedoraQt:master Mar 26, 2020
@mitya57 mitya57 deleted the fix-crash-on-32-bit branch March 26, 2020 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants