Possible issue in dlls/d3dx9_36/surface.c?

[Gerald Pfeifer]

Matteo et al,

there is some code in dlls/d3dx9_36/surface.c which GCC struggles with
(in the sense of warning about it), and so do I, so I am wondering whether
you can have a look?

Specifically, in point_filter_simple_data we have:

DWORD val = 0, pixel;

/* extract source color components */
       if(srcformat->type == FORMAT_ARGB) {
           pixel = dword_from_bytes(srcptr, srcformat->bytes_per_pixel);
           get_relevant_argb_components(&conv_info, pixel, channels);
       }

So, pixel is uninitialized, except when we have FORMAT_ARGB.

However, directly below we then have

/* recombine the components */
       if(destformat->type == FORMAT_ARGB) val = make_argb_color(&conv_info, channels);

if(colorkey) {
           get_relevant_argb_components(&ck_conv_info, pixel, channels);

where pixel is passed to get_relevant_argb_components.  This is guarded
by colorkey, a parameter to this function, so indeed I can see pixel being
unused here.

(I'd naively propose a patch like the one below; this may not be 
sufficient though.)

Gerald

diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c
index a3e6e68..cc36818 100644
--- a/dlls/d3dx9_36/surface.c
+++ b/dlls/d3dx9_36/surface.c
@@ -701,7 +701,7 @@ static void copy_simple_data(CONST BYTE *src, UINT srcpitch, POINT srcsize,
         DWORD val = 0;

for(x = 0;x < minwidth;x++) {
-            DWORD pixel;
+            DWORD pixel = 0xFFFFFFFF;

/* extract source color components */
             if(srcformat->type == FORMAT_ARGB) {
@@ -768,7 +768,7 @@ static void point_filter_simple_data(CONST BYTE *src, UINT srcpitch, POINT srcsi

for(x = 0;x < destsize.x;x++) {
             const BYTE *srcptr = bufptr + (x * srcsize.x / destsize.x) * srcformat->bytes_per_pixel;
-            DWORD val = 0, pixel;
+            DWORD val = 0, pixel = 0xFFFFFFFF;

/* extract source color components */
             if(srcformat->type == FORMAT_ARGB) {

[Matteo Bruni]

On Sat, 2011-04-02 at 23:54 +0200, Gerald Pfeifer wrote:
> Matteo et al,
>
> there is some code in dlls/d3dx9_36/surface.c which GCC struggles with
> (in the sense of warning about it), and so do I, so I am wondering whether
> you can have a look?
>
> Specifically, in point_filter_simple_data we have:
>    DWORD val = 0, pixel;
>
>    /* extract source color components */
>    if(srcformat->type == FORMAT_ARGB) {
>        pixel = dword_from_bytes(srcptr, srcformat->bytes_per_pixel);
>        get_relevant_argb_components(&conv_info, pixel, channels);
>    }
>
> So, pixel is uninitialized, except when we have FORMAT_ARGB.
>
> However, directly below we then have
>    /* recombine the components */
>    if(destformat->type == FORMAT_ARGB) val = make_argb_color(&conv_info, channels);
>
>    if(colorkey) {
>        get_relevant_argb_components(&ck_conv_info, pixel, channels);
>
> where pixel is passed to get_relevant_argb_components.  This is guarded
> by colorkey, a parameter to this function, so indeed I can see pixel being
> unused here.

Currently that is not a real problem, as every pixel format has type
FORMAT_ARGB (you can see that from the formats array in util.c). Surely
the code could be nicer though.
> (I'd naively propose a patch like the one below; this may not be 
> sufficient though.)
>
> Gerald
>
> diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c
> index a3e6e68..cc36818 100644
> --- a/dlls/d3dx9_36/surface.c
> +++ b/dlls/d3dx9_36/surface.c
> @@ -701,7 +701,7 @@ static void copy_simple_data(CONST BYTE *src, UINT srcpitch, POINT srcsize,
>          DWORD val = 0;
>      for(x = 0;x < minwidth;x++) {
> -
>        DWORD pixel;
> -
>        DWORD pixel = 0xFFFFFFFF;
>
>        /* extract source color components */
>        if(srcformat->type == FORMAT_ARGB) {
>
> @@ -768,7 +768,7 @@ static void point_filter_simple_data(CONST BYTE *src, UINT srcpitch, POINT srcsi
>      for(x = 0;x < destsize.x;x++) {
>          const BYTE *srcptr = bufptr + (x * srcsize.x / destsize.x) * srcformat->bytes_per_pixel;
> -
>        DWORD val = 0, pixel;
> -
>        DWORD val = 0, pixel = 0xFFFFFFFF;
>
>        /* extract source color components */
>        if(srcformat->type == FORMAT_ARGB) {

That would silence the warnings but it doesn't look to me as the right
fix. I think the easiest is to just move the format type checks from
copy_simple_data and point_filter_simple_data to
D3DXLoadSurfaceFromMemory (which is the only caller) and bail out if the
source or the destination format is not FORMAT_ARGB.

I can prepare a patch for that, if you don't feel like giving it a try.
Thanks for the heads-up in any case. :)