Re: wine/dlls/shell32 shell32_main.c

[Rein Klazes]

On Fri, 20 Jun 2003 21:01:57 -0500, you wrote:
> Modified files:
>   dlls/shell32   : shell32_main.c
>
> Log message:
>   Jason Edmeades <us(a)the-edmeades.demon.co.uk>
>   SHGetFileInfo should tolerate null pointers.
>
> Patch: http://cvs.winehq.com/patch.py?root=/home/winehq/opt/cvs-commit&id=8543

This breaks the file dialog in Agent news reader: the directory tree is
blank. 

Precisely the cause is this part:


| @@ -297,6 +299,9 @@
|  	  return 0;
|        }
|  
| +      /* psfi is NULL normally to query EXE type, if not none of the below makes 
| +           sense anyway. Windows allows this and just returns FALSE              */
| +      if (psfi != NULL) return FALSE;

this is obviously wrong (psfi is dereferenced in the remainder of the
function). I just wonder if it should be removed, or is here a typo and
should != be  == ?

Rein.
-- 
Rein Klazes
rklazes(a)xs4all.nl

wine/dlls/shell32 shell32_main.c

[BiGgUn]

Hi,
> | @@ -297,6 +299,9 @@
> |    return 0;
> |        }
> |
> | +      /* psfi is NULL normally to query EXE type, if not none of the
> below makes
> | +           sense anyway. Windows allows this and just returns FALSE
> */
> | +      if (psfi != NULL) return FALSE;
>
> this is obviously wrong (psfi is dereferenced in the remainder of the
> function). I just wonder if it should be removed, or is here a typo and
> should != be  == ?

I think this instruction should be removed because it has no sense.  When
you query EXE type, this instruction is never reached. A value is always
returned , psfi == NULL or not. In other cases ( file dialog icons for
instance) , this instruction makes things going wrong.

Stephan

wine/dlls/shell32 shell32_main.c

[Ann and Jason Edmeades]

> | +      /* psfi is NULL normally to query EXE type, if not none of the
> below makes
> | +           sense anyway. Windows allows this and just returns FALSE
> */
> | +      if (psfi != NULL) return FALSE;
> this is obviously wrong (psfi is dereferenced in the remainder of the
> function). I just wonder if it should be removed, or is here a typo and
> should != be  == ?

Its a typo - I suspect I coded and tested it as if (!psfi) return false and
'tidied' it up incorrectly at the last minute. If should (obviously!) say
"if (psfi == null) return FALSE;"

Sincere apologies for this - repeat after me... I must re-test on the very
final version of the patch....:-)

Could I ask you to patch it though, as I will not get to do it until Tues
otherwise (I'll do it then if you havent before).

Jason

wine/dlls/shell32 shell32_main.c

[Rein Klazes]

On Fri, 4 Jul 2003 18:31:16 +0100, you wrote:
> > | +      /* psfi is NULL normally to query EXE type, if not none of the
> > below makes
> > | +           sense anyway. Windows allows this and just returns FALSE
> > */
> > | +      if (psfi != NULL) return FALSE;
> > this is obviously wrong (psfi is dereferenced in the remainder of the
> > function). I just wonder if it should be removed, or is here a typo and
> > should != be  == ?
>
> Its a typo - I suspect I coded and tested it as if (!psfi) return false and
> 'tidied' it up incorrectly at the last minute. If should (obviously!) say
> "if (psfi == null) return FALSE;"
>
> Sincere apologies for this - repeat after me... I must re-test on the very
> final version of the patch....:-)
>
> Could I ask you to patch it though, as I will not get to do it until Tues
> otherwise (I'll do it then if you havent before).

Sure. I will fix the comment as well, I think it did describe the wrong
code accurately. 

Rein.
-- 
Rein Klazes
rklazes(a)xs4all.nl