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

Some trivial XCB bugs #786

Closed
jcowgill opened this issue Jan 26, 2015 · 3 comments · Fixed by #797
Closed

Some trivial XCB bugs #786

jcowgill opened this issue Jan 26, 2015 · 3 comments · Fixed by #797

Comments

@jcowgill
Copy link
Contributor

Found with clang static code analyzer:

https://github.com/LaurentGomila/SFML/blob/master/src/SFML/Window/Unix/InputImpl.cpp#L205

src/SFML/Window/Unix/InputImpl.cpp:205:31: warning: Value stored to 'result' is never read
        case Mouse::Left:     result = pointer->mask & XCB_BUTTON_MASK_1;
                              ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/SFML/Window/Unix/InputImpl.cpp:206:31: warning: Value stored to 'result' is never read
        case Mouse::Right:    result = pointer->mask & XCB_BUTTON_MASK_3;
                              ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/SFML/Window/Unix/InputImpl.cpp:207:31: warning: Value stored to 'result' is never read
        case Mouse::Middle:   result = pointer->mask & XCB_BUTTON_MASK_2;
                              ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

https://github.com/LaurentGomila/SFML/blob/master/src/SFML/Window/Unix/WindowImplX11.cpp#L538

src/SFML/Window/Unix/WindowImplX11.cpp:538:73: warning: Use of memory after it is freed
        err() << "Failed to set the window's icon: Error code " << (int)errptr->error_code << std::endl;
                                                                        ^~~~~~~~~~~~~~~~~~
@LaurentGomila
Copy link
Member

Thanks.

The first one is less obvious: the "break" statements are missing. It would be much better like this:

    // Get pointer mask
    xcb_query_pointer_reply_t* pointer = xcb_query_pointer_reply(connection, xcb_query_pointer(connection, XDefaultRootWindow(display)), NULL);
    uint16_t mask = pointer->mask;
    free(pointer);

    // Close the connection with the X server
    CloseDisplay(display);

    switch (button)
    {
        case Mouse::Left:     return mask & XCB_BUTTON_MASK_1;
        case Mouse::Right:    return mask & XCB_BUTTON_MASK_3;
        case Mouse::Middle:   return mask & XCB_BUTTON_MASK_2;
        case Mouse::XButton1: return false; // not supported by X
        case Mouse::XButton2: return false; // not supported by X
        default:              return false;
    }

@kimci86
Copy link
Contributor

kimci86 commented Jan 26, 2015

@LaurentGomila
I guess you meant to return mask & ... instead of pointer->mask & ...

@LaurentGomila
Copy link
Member

Oops 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants