Re: [ddraw] Fix bug 3487 take 2

[Peter Berg Larsen]

On Sun, 9 Oct 2005, Lionel Ulmer wrote:
> Well, while your patch was lying in the moderation queue, I sent what I 
> feel is a better solution to this problem (which fixes also a severe 
> reference counting issue).

I had more than one goal with the patch, more below.
> Could you try it and tell if it fixes the problem ?

It, the patch on the bug page, does (e.i. without the locking). Could I 
suggest a comment in the code.
> PS: and Raphael's patch while not fixing your bug is not technically 
> wrong as Windows checks for the surface description pointer being 
> non-NULL

Nor did I say that; just that it had nothing to with bug 3487 as the
subject said it had.

On Sun, 9 Oct 2005, Christian Costa wrote:
> This does not work if the destination size is smaller than the struct size or 
> source size is greater than destination size.
> Even worst, if source size is greater than struct size, 
> sizeof(*(to))-__fromsize is < 0.
> Why don't you just add the "if (to = from) break;" statement ?

I wrote the patch, not because of bug 3487, but because, as I
wrote in take 1:
> >

Pure luck to find this one. I didnt understand what was going on;
and rewrote it. I am still uncertain about invariants wrt. sizeof
and dwSize.
<<

It is impossible to follow. Ok; in the begining it was only a memcpy, then 
because the debug was gabled an memset was added. Then bug 2070 states the 
the (to)->dwSize could be too high. Rather than just adding a if 
statement, I decided that it needed a complete rewrite.

The invariants then is:
sizeof(*to) <= to->dwSize
sizeof(*from) == from->dwSize

Why is the old code testing the sizeof(*to) < to->dwsize for the memset 
case only? Why first memset and then copy over it? Why use 3 variables?

The goal was the do the memcpy and then memset the rest. The second goal 
was to bark if the invariants were broken. The third goal was to have the 
compiler remove any ifs when debugging was turned off.

I tried play around with

if(to != from) {
    ....
}

or

do {
    if (to!=from) {
    ...
    }
} while(0)

but found the

if (to!=from) break

cleanest.

The first ERR should have been a warn though.

Peter