Problem in dlls/ddraw/dsufrace/main.c

[Sylvain Petreolle]

Trying to run Revolt Demo, the program halts saying DDERR_NOTFOUND in 
Main_DirectDrawSurface_GetAttachedSurface

Looking in the source, we see that :

found=NULL;
for (surf = This->attached; surf != NULL; surf = surf->next_attached){
... // surf is not modified into that
if (...) found=surf;
}

if (found == NULL) {
TRACE("Did not find any valid surface\n");
return DDERR_NOTFOUND;
}

Looks like the only possible return value of this is DDERR_NOTFOUND,
because of the for(...; surf !=NULL ; ...)

Or am I writing complete bullshit ?

=====
Sylvain Petreolle
[e-mail] 
Fight against Spam ! http://www.euro.cauce.org/en/index.html
ICQ #170597259

"Don't think you are. Know you are." Morpheus in Matrix, chapter 15.

___________________________________________________________
Do You Yahoo!? -- Une adresse @yahoo.fr gratuite et en français !
Yahoo! Mail : http://fr.mail.yahoo.com

[Lionel Ulmer]

> found=NULL;
> for (surf = This->attached; surf != NULL; surf = surf->next_attached){
> ... // surf is not modified into that
> if (...) found=surf;
> }
> if (found == NULL) {
> TRACE("Did not find any valid surface\n");
> return DDERR_NOTFOUND;
> }

Well, we loop on 'surf' (which will be NULL at the end of the loop) and test
'found' (that won't be NULL if a matching surface was found). So no, I do
not see any problem here.

Lionel

[Sylvain Petreolle]

OK.
So in the case found=surf=NULL and we disable the test that exits with
an error,
should the part of the function after the test fail or not ?
> Well, we loop on 'surf' (which will be NULL at the end of the loop)
> and test
> 'found' (that won't be NULL if a matching surface was found). So no,
> I do
> not see any problem here.

=====
Sylvain Petreolle
[e-mail] 
Fight against Spam ! http://www.euro.cauce.org/en/index.html
ICQ #170597259

"Don't think you are. Know you are." Morpheus in Matrix, chapter 15.

___________________________________________________________
Do You Yahoo!? -- Une adresse @yahoo.fr gratuite et en français !
Yahoo! Mail : http://fr.mail.yahoo.com

[Lionel Ulmer]

On Tue, Jan 07, 2003 at 10:11:02PM +0100, Sylvain Petreolle wrote:
> OK.
> So in the case found=surf=NULL and we disable the test that exits with
> an error, should the part of the function after the test fail or not ?

Well, according to what the MSDN tells us, if no surface is found or if more
than one surface matching the requirements are found, the function should
return DDERR_NOTFOUND.

So I do not really understand what you mean. If you suspect this error to be
the cause of the game not to work, you should rather look if it's normal or
not to not find the requested attached surface.

Lionel

[Sylvain Petreolle]

Well, I said the wrong words to explain me :/.
Disabling the test in our code, will the function fail or not ?

The reason I write is that : in fact, I disabled the failing test and
the game (Re-Volt Demo) begins to start.
> Well, according to what the MSDN tells us, if no surface is found or
> if more
> than one surface matching the requirements are found, the function
> should
> return DDERR_NOTFOUND.
>
> So I do not really understand what you mean. If you suspect this
> error to be
> the cause of the game not to work, you should rather look if it's
> normal or
> not to not find the requested attached surface.

=====
Sylvain Petreolle
[e-mail] 
Fight against Spam ! http://www.euro.cauce.org/en/index.html
ICQ #170597259

"Don't think you are. Know you are." Morpheus in Matrix, chapter 15.

___________________________________________________________
Do You Yahoo!? -- Une adresse @yahoo.fr gratuite et en français !
Yahoo! Mail : http://fr.mail.yahoo.com

[Lionel Ulmer]

> Well, I said the wrong words to explain me :/.
> Disabling the test in our code, will the function fail or not ?

It will crash in the AddRef code. Better to try to do something like that :

if (found == NULL) {
       TRACE("Did not find any valid surface\n");
       *ppSurface = NULL;
       return DD_OK;
    }

To be 100 %, the best would be for one to do a test in real Windows and
check what happens when asking for a non-existant attached surface and check
what happens (ie DD_OK with ppSurface being NULL or DDERR_NOTFOUND).
> The reason I write is that : in fact, I disabled the failing test and
> the game (Re-Volt Demo) begins to start.

Well, as said before, try to see if this call SHOULD fail or if it's a bug
that we do not find this attached surface.

Even better would be to disassemble the code handling the return of this
function to see what kind of stuff the game expects.

Lionel

[Sylvain Petreolle]

> It will crash in the AddRef code. Better to try to do something like
> that :
> if (found == NULL) {
>    TRACE("Did not find any valid surface\n");
>    *ppSurface = NULL;
>    return DD_OK;
> }

Thanks, I will try it.
But with the test disabled, I get no crash in AddRef (??).
See the following resuslts :
trace:ddraw:Main_DirectDrawSurface_GetAttachedSurface
(0x402a9418)->Looking for caps: 4,0,1,1 output: 0x741ed0
   Caps are : DDSCAPS_BACKBUFFER
trace:ddraw:Main_DirectDrawSurface_GetAttachedSurface Surface:
(0x402a9b38) caps: 1000601c,0,0,0
   Surface caps are : DDSCAPS_BACKBUFFER DDSCAPS_COMPLEX DDSCAPS_FLIP
DDSCAPS_3DDEVICE DDSCAPS_VIDEOMEMORY DDSCAPS_LOCALVIDMEM
trace:ddraw:Main_DirectDrawSurface_GetAttachedSurface Returning surface
0x402a9b38 with description :
 - DDSD_CAPS : DDSCAPS_BACKBUFFER DDSCAPS_COMPLEX DDSCAPS_FLIP
DDSCAPS_3DDEVICE DDSCAPS_VIDEOMEMORY DDSCAPS_LOCALVIDMEM
 - DDSD_HEIGHT : 480
 - DDSD_WIDTH : 640
 - DDSD_PITCH : 1280
 - DDSD_LPSURFACE : 0x424b0000
 - DDSD_PIXELFORMAT : ( DDPF_RGB , RGB bits: 16,  R f800 G 07e0 B 001f)
trace:ddraw:Main_DirectDrawSurface_AddRef (0x402a9b38)->(): increasing
from 1
trace:ddraw:Main_DirectDrawSurface_AddAttachedSurface
(0x402a9b38)->(0x402a90e8)
trace:ddraw:Main_DirectDrawSurface_AddRef (0x402a90e8)->(): increasing
from 1
The only err: that I get inddraw trace are unhandled dwRenderStateType.

Argh, cant do it for now.
> To be 100 %, the best would be for one to do a test in real Windows
> and
> check what happens when asking for a non-existant attached surface
> and check
> what happens (ie DD_OK with ppSurface being NULL or DDERR_NOTFOUND).
> > The reason I write is that : in fact, I disabled the failing test
>
> and
> > the game (Re-Volt Demo) begins to start.
>
> Well, as said before, try to see if this call SHOULD fail or if it's
> a bug
> that we do not find this attached surface.
>
> Even better would be to disassemble the code handling the return of
> this
> function to see what kind of stuff the game expects.
>                     Lionel
>
> -- 
>    	 Lionel Ulmer - http://www.bbrox.org/

=====
Sylvain Petreolle
[e-mail] 
Fight against Spam ! http://www.euro.cauce.org/en/index.html
ICQ #170597259

"Don't think you are. Know you are." Morpheus in Matrix, chapter 15.

___________________________________________________________
Do You Yahoo!? -- Une adresse @yahoo.fr gratuite et en français !
Yahoo! Mail : http://fr.mail.yahoo.com

[Lionel Ulmer]

> trace:ddraw:Main_DirectDrawSurface_GetAttachedSurface
> (0x402a9418)->Looking for caps: 4,0,1,1 output: 0x741ed0
>    Caps are : DDSCAPS_BACKBUFFER
> trace:ddraw:Main_DirectDrawSurface_GetAttachedSurface Surface:
> (0x402a9b38) caps: 1000601c,0,0,0
>    Surface caps are : DDSCAPS_BACKBUFFER DDSCAPS_COMPLEX DDSCAPS_FLIP
> DDSCAPS_3DDEVICE DDSCAPS_VIDEOMEMORY DDSCAPS_LOCALVIDMEM
> trace:ddraw:Main_DirectDrawSurface_GetAttachedSurface Returning surface
> 0x402a9b38 with description :
> - DDSD_CAPS : DDSCAPS_BACKBUFFER DDSCAPS_COMPLEX DDSCAPS_FLIP
>
> DDSCAPS_3DDEVICE DDSCAPS_VIDEOMEMORY DDSCAPS_LOCALVIDMEM
> - DDSD_HEIGHT : 480
> - DDSD_WIDTH : 640
> - DDSD_PITCH : 1280
> - DDSD_LPSURFACE : 0x424b0000
> - DDSD_PIXELFORMAT : ( DDPF_RGB , RGB bits: 16,  R f800 G 07e0 B 001f)

Well, you completely lost me here... Could you restate your problem from the
beginning ? Because well, in your trace, I see that you actually FOUND a
valid attached surface. So the 'if (found == NULL)' test will be FALSE and
thus whether this code is in or not would not change anything at all.

So I doubt that removing this piece of code would change anything in the
application's behaviour.

The only thing I could see would be that you were running with a quite old
CVS version and when you ugraded you got one fix I did on the
GetAttachedSurface code to handle some problem we had with useless caps in
the DDSCAPS structure being tested whereas they should not have been.

Lionel

[Sylvain Petreolle]

no problem any more with this code when I updated my CVS (aged from
Tuesday morning :) )
black magic ;)

--- Lionel Ulmer  a écrit : > >
trace:ddraw:Main_DirectDrawSurface_GetAttachedSurface
> > (0x402a9418)->Looking for caps: 4,0,1,1 output: 0x741ed0
> >    Caps are : DDSCAPS_BACKBUFFER
> > trace:ddraw:Main_DirectDrawSurface_GetAttachedSurface Surface:
> > (0x402a9b38) caps: 1000601c,0,0,0
> >    Surface caps are : DDSCAPS_BACKBUFFER DDSCAPS_COMPLEX
>
> DDSCAPS_FLIP
> > DDSCAPS_3DDEVICE DDSCAPS_VIDEOMEMORY DDSCAPS_LOCALVIDMEM
> > trace:ddraw:Main_DirectDrawSurface_GetAttachedSurface Returning
>
> surface
> > 0x402a9b38 with description :
> > - DDSD_CAPS : DDSCAPS_BACKBUFFER DDSCAPS_COMPLEX DDSCAPS_FLIP
> >
> > DDSCAPS_3DDEVICE DDSCAPS_VIDEOMEMORY DDSCAPS_LOCALVIDMEM
> > - DDSD_HEIGHT : 480
> > - DDSD_WIDTH : 640
> > - DDSD_PITCH : 1280
> > - DDSD_LPSURFACE : 0x424b0000
> > - DDSD_PIXELFORMAT : ( DDPF_RGB , RGB bits: 16,  R f800 G 07e0 B
>
> 001f)
>
> Well, you completely lost me here... Could you restate your problem
> from the
> beginning ? Because well, in your trace, I see that you actually
> FOUND a
> valid attached surface. So the 'if (found == NULL)' test will be
> FALSE and
> thus whether this code is in or not would not change anything at all.
>
> So I doubt that removing this piece of code would change anything in
> the
> application's behaviour.
>
> The only thing I could see would be that you were running with a
> quite old
> CVS version and when you ugraded you got one fix I did on the
> GetAttachedSurface code to handle some problem we had with useless
> caps in
> the DDSCAPS structure being tested whereas they should not have been.
>       Lionel
>
> -- 
>    	 Lionel Ulmer - http://www.bbrox.org/

=====
Sylvain Petreolle
[e-mail] 
Fight against Spam ! http://www.euro.cauce.org/en/index.html
ICQ #170597259

"Don't think you are. Know you are." Morpheus, in "Matrix".

___________________________________________________________
Do You Yahoo!? -- Une adresse @yahoo.fr gratuite et en français !
Yahoo! Mail : http://fr.mail.yahoo.com

[Sylvain Petreolle]

--- Lionel Ulmer  a écrit : > > Well, I said the
wrong words to explain me :/.
> > Disabling the test in our code, will the function fail or not ?
>
> It will crash in the AddRef code. Better to try to do something like
> that :
> if (found == NULL) {
>    TRACE("Did not find any valid surface\n");
>    *ppSurface = NULL;
>    return DD_OK;
> }

I get the same behaviour with this code enabled as with the whole test disabled.

=====
Sylvain Petreolle
[e-mail] 
Fight against Spam ! http://www.euro.cauce.org/en/index.html
ICQ #170597259

"Don't think you are. Know you are." Morpheus in Matrix, chapter 15.

___________________________________________________________
Do You Yahoo!? -- Une adresse @yahoo.fr gratuite et en français !
Yahoo! Mail : http://fr.mail.yahoo.com