[ros-dev] Regression fixing

[Aleksey Bragin]

On May 1, 2010, at 7:57 AM, James Tabor wrote:
> I'm reacting to when it gets broken! I could be wrong and when I am I
> do admit it openly! But, you need to read bug 5265. I guess we can
> revert all the gdi batch code to fix it. Yes it fixes it but still the
> real problem persists, what happen to the TEB?
This is exactly the problematic behavior: Why enable GDI batch code  
if it does NOT work?  Fix underlying bug *first* and then enable the  
code. Especially considering that GDI batch may be seen as  
optimization, not as a vital code needed for rendering. Is it?

And honestly, you are trying to find a bug in the wrong direction, in  
my opinion.
Analyze it logically: somehow, TEB appears to work everywhere except  
for your GDI batch code. And, one of the hackfixs you committed in  
46414 is around this code:
        for (; GdiBatchCount > 0; GdiBatchCount--)
        {
            ULONG Size;
             // Process Gdi Batch!
            Size = GdiFlushUserBatch(pDC, (PGDIBATCHHDR) pHdr);
            if (!Size) break;
            pHdr += Size;
         }
You "protected" pHdr pointer dereference inside GdiFlushUserBatch()  
by wrapping it into SEH.
Now let's call Captain Obvious again to help and think, what is more  
probable: Mysterious TEB invalidation, or GdiBatchCount not matching  
the size of pHdr array? So that the reading goes beyond the buffer  
and certainly faults (and you hack-catch this fault in SEH to pretend  
you caught the mysterious kernel bug).
I suspect the latter, because it might be anything - race condition,  
a bug in the code, a buffer overflow, uninitialized variable access,  
or you're trying to flush the batch queue of an already dead thread.

That's just a glance over the bug, without even looking to the code -  
just reading the diff was enough.


> http://www.reactos.org/bugzilla/show_bug.cgi?id=5265
>
> This bug is due to a hax fix I did to help 5265!
> http://www.reactos.org/bugzilla/show_bug.cgi?id=5314
>
> Might I add, no one has followed up on any of my suggestions.
That's why you decided to crash in the non-working code and wait for  
people's reaction? Not good.

>
>>
>>> Look at this as a signal warning before the ship hits the  
>>> reef.......
>>> The ship has turn into the reef!
>>
>> No need to give false signals here, they often lead to loss of  
>> investment
>> (----->example from stock trading area<-----).
> Trying to make money from ReactOS is a bad idea and thinking about
> money from ReactOS, makes this idea delusional.
Your weed is very nice, please share.