Skip to content

[0.4.x] libvisual: Fix conversion of color to uint32#142

Merged
hartwork merged 1 commit into0.4.xfrom
0.4.x-fix-conversion-of-color-to-uint32
Dec 5, 2022
Merged

[0.4.x] libvisual: Fix conversion of color to uint32#142
hartwork merged 1 commit into0.4.xfrom
0.4.x-fix-conversion-of-color-to-uint32

Conversation

@hartwork
Copy link
Copy Markdown
Member

@hartwork hartwork commented Dec 2, 2022

Original report: https://sourceforge.net/p/libvisual/bugs/18/

#include <stdint.h>
#include <stdio.h>

int main() {
        int r = 0x11;
        int g = 0x22;
        int b = 0x33;

        uint32_t colors;

        colors = (256 << 24) | (r << 16) | (g << 8) | b;
        printf("Old: 0x%08x\n", colors);

        colors = (255 << 24) | (r << 16) | (g << 8) | b;
        printf("New: 0x%08x\n", colors);

        return 0;
}
# gcc main.c && ./a.out
main.c: In function ‘main’:
main.c:11:23: warning: result of ‘256 << 24’ requires 34 bits to represent, but ‘int’ only has 32 bits [-Wshift-overflow=]
   11 |         colors = (256 << 24) | (r << 16) | (g << 8) | b;
      |                       ^~
Old: 0x00112233
New: 0xff112233

CC @dcb314

@hartwork hartwork added the bug label Dec 2, 2022
@hartwork hartwork added this to the 0.4.1 milestone Dec 2, 2022
Copy link
Copy Markdown

@grandchild grandchild left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may consider writing 0xff instead of 255 to make it super obvious that the intent is to fill the MSB with all-ones.

Clang said:
> [..]/libvisual/lv_color.c:263:16: error: signed shift result (0x100000000) requires 34 bits to represent, but 'int' only has 32 bits [-Werror,-Wshift-overflow]
>         colors = (256 << 24) |
>                   ~~~ ^  ~~

A closer look reveals that 255 (0xFF) is the maximum alpha value
and that 256 (0x100) should have been 255 in the first place.

Original report at https://sourceforge.net/p/libvisual/bugs/18/
@hartwork hartwork force-pushed the 0.4.x-fix-conversion-of-color-to-uint32 branch from 92c90b4 to 2d2e608 Compare December 2, 2022 23:48
@hartwork
Copy link
Copy Markdown
Member Author

hartwork commented Dec 2, 2022

You may consider writing 0xff instead of 255 to make it super obvious that the intent is to fill the MSB with all-ones.

No objections, done. Thanks for the review!

@hartwork hartwork merged commit 440aecf into 0.4.x Dec 5, 2022
@hartwork hartwork deleted the 0.4.x-fix-conversion-of-color-to-uint32 branch December 5, 2022 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants