Possible issue in dlls/d3dx9_36/surface.c?

2011-04-03 Thread 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 = 0x;
 
 /* 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 = 0x;
 
 /* extract source color components */
 if(srcformat-type == FORMAT_ARGB) {




Re: Possible issue in dlls/d3dx9_36/surface.c?

2011-04-03 Thread 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 = 0x;
  
  /* 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 = 0x;
  
  /* 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. :)