Gerald Pfeifer [email protected] wrote:
--- a/include/d3dtypes.h +++ b/include/d3dtypes.h @@ -53,7 +53,7 @@ typedef LONG D3DFIXED; #define RGBA_GETRED(rgb) (((rgb) >> 16) & 0xff) #define RGBA_GETGREEN(rgb) (((rgb) >> 8) & 0xff) #define RGBA_GETBLUE(rgb) ((rgb) & 0xff) -#define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((a) << 24) | ((r) << 16) | ((g) << 8) | (b))) +#define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((unsigned)(a) << 24) | ((r) << 16) | ((g) << 8) | (b)))
This is a public header, probably you need to fix the callers of this macro instead.
On 6 September 2016 at 08:41, Dmitry Timoshkov [email protected] wrote:
Gerald Pfeifer [email protected] wrote:
--- a/include/d3dtypes.h +++ b/include/d3dtypes.h @@ -53,7 +53,7 @@ typedef LONG D3DFIXED; #define RGBA_GETRED(rgb) (((rgb) >> 16) & 0xff) #define RGBA_GETGREEN(rgb) (((rgb) >> 8) & 0xff) #define RGBA_GETBLUE(rgb) ((rgb) & 0xff) -#define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((a) << 24) | ((r) << 16) | ((g) << 8) | (b))) +#define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((unsigned)(a) << 24) | ((r) << 16) | ((g) << 8) | (b)))
This is a public header, probably you need to fix the callers of this macro instead.
Signed overflow is undefined, so this change is compatible. It's also unlikely to be an issue in practice on any architecture we support. I might have preferred something similar to D3DCOLOR_ARGB though.
That's not to say the users of the macro don't have room for improvement.
Hi guys,
On Tue, 6 Sep 2016, Henri Verbeet wrote:
On 6 September 2016 at 08:41, Dmitry Timoshkov [email protected] wrote:
--- a/include/d3dtypes.h +++ b/include/d3dtypes.h @@ -53,7 +53,7 @@ typedef LONG D3DFIXED; #define RGBA_GETRED(rgb) (((rgb) >> 16) & 0xff) #define RGBA_GETGREEN(rgb) (((rgb) >> 8) & 0xff) #define RGBA_GETBLUE(rgb) ((rgb) & 0xff) -#define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((a) << 24) | ((r) << 16) | ((g) << 8) | (b))) +#define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((unsigned)(a) << 24) | ((r) << 16) | ((g) << 8) | (b)))
This is a public header, probably you need to fix the callers of this macro instead.
Signed overflow is undefined, so this change is compatible. It's also unlikely to be an issue in practice on any architecture we support. I might have preferred something similar to D3DCOLOR_ARGB though.
That's not to say the users of the macro don't have room for improvement.
I noticed this has not been committed (nor rejected apparently). but after reverting it locally, I do not see the warnings that triggered this any longer.
Did one of you commit and alternative fix?
Gerald
On 26 February 2017 at 15:22, Gerald Pfeifer [email protected] wrote:
On Tue, 6 Sep 2016, Henri Verbeet wrote:
On 6 September 2016 at 08:41, Dmitry Timoshkov [email protected] wrote:
--- a/include/d3dtypes.h +++ b/include/d3dtypes.h @@ -53,7 +53,7 @@ typedef LONG D3DFIXED; #define RGBA_GETRED(rgb) (((rgb) >> 16) & 0xff) #define RGBA_GETGREEN(rgb) (((rgb) >> 8) & 0xff) #define RGBA_GETBLUE(rgb) ((rgb) & 0xff) -#define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((a) << 24) | ((r) << 16) | ((g) << 8) | (b))) +#define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((unsigned)(a) << 24) | ((r) << 16) | ((g) << 8) | (b)))
This is a public header, probably you need to fix the callers of this macro instead.
Signed overflow is undefined, so this change is compatible. It's also unlikely to be an issue in practice on any architecture we support. I might have preferred something similar to D3DCOLOR_ARGB though.
That's not to say the users of the macro don't have room for improvement.
I noticed this has not been committed (nor rejected apparently). but after reverting it locally, I do not see the warnings that triggered this any longer.
Did one of you commit and alternative fix?
Commit 060ea15ae2c1604d51d3df1460e99c7d6e6211cf should have avoided the issue.
On Sun, 26 Feb 2017, Henri Verbeet wrote:
-#define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((a) << 24) | ((r) << 16) | ((g) << 8) | (b))) +#define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((unsigned)(a) << 24) | ((r) << 16) | ((g) << 8) | (b)))
Commit 060ea15ae2c1604d51d3df1460e99c7d6e6211cf should have avoided the issue.
Yes, indeed, that looks like it. Thank you, Henri!
Gerald
-
Dmitry Timoshkov -
Gerald Pfeifer -
Henri Verbeet