bug in vfatfs.sys?

[Oriol]

I get an error while trying to install Firefox 2.0 on a bootcd
compilation, finally i found the possibly bug is in vfatfs.sys i
modified some lines but is not my code.....it seems to install Firefox.
Also, testing firefox i found problems with gdi and also done some
modifications, still testing them..
Here is my patch..NO WARRANTY he he..

[James Tabor]

Hi,
I can only write about the gdi code changes.

1st dll/win32/gdi32/misc/misc.c looks good!
2nd the dc.c changes,,, ATM the support code for dirty bit sets is not
inplace, but the patch is good.

I will apply your patch ASAP for the GDI changes, it does look good.

Thanks,
James

On Jan 3, 2008 3:41 PM, Oriol <oripipa(a)yahoo.es> wrote:
>   I get an error while trying to install Firefox 2.0 on
> a bootcd
>  compilation, finally i found the possibly bug is in vfatfs.sys i
>  modified some lines but is not my code.....it seems to install Firefox.
>  Also, testing firefox i found problems with gdi and also done some
>  modifications, still testing them..
>  Here is my patch..NO WARRANTY he he..
>
>

[Oriol]

i current have these modifications to the vfatfs,I made other changes
but now testing with this diff..
If still not working i'll look other changes i made..

[Alex Ionescu]

On 3-Jan-08, at 4:41 PM, Oriol wrote:

>   I get an error while trying to install Firefox 2.0 on
> a bootcd
>  compilation, finally i found the possibly bug is in vfatfs.sys i
>  modified some lines but is not my code.....it seems to install  
>  Firefox.
>  Also, testing firefox i found problems with gdi and also done some
>  modifications, still testing them..
>  Here is my patch..NO WARRANTY he he..
>
>  Index: tools/rbuild/configuration.cpp
>  ===================================================================
>  --- tools/rbuild/configuration.cpp	(revision 31587)
>  +++ tools/rbuild/configuration.cpp	(working copy)
>  @@ -28,7 +28,7 @@
>   	CheckDependenciesForModuleOnly = false;
>   	CompilationUnitsEnabled = true;
>   	MakeHandlesInstallDirectories = false;
>  -	GenerateProxyMakefilesInSourceTree = false;
>  +	GenerateProxyMakefilesInSourceTree = true;
>   	InstallFiles = false;
>   	UseConfigurationInPath = false;
>   	UseVSVersionInPath = false; 
What? Why? How does this help fix a VFAT bug?

>   Index: ntoskrnl/include/internal/mm.h
>  ===================================================================
>  --- ntoskrnl/include/internal/mm.h	(revision 31587)
>  +++ ntoskrnl/include/internal/mm.h	(working copy)
>  @@ -103,7 +103,7 @@
>   /*
>    * Maximum size of the kmalloc area (this is totally arbitary)
>    */
>  -#define MM_KERNEL_MAP_SIZE                  (16*1024*1024)
>  +#define MM_KERNEL_MAP_SIZE                  (64*1024*1024)
>   #define MM_KERNEL_MAP_BASE                  (0xf0c00000)
>
>   /* 
What? Nobody uses this value anymore, and why would increasing it to  
64MB help? These values need to go away.

>   Index: ntoskrnl/mm/rpoolmgr.h
>  ===================================================================
>  --- ntoskrnl/mm/rpoolmgr.h	(revision 31587)
>  +++ ntoskrnl/mm/rpoolmgr.h	(working copy)
>  @@ -32,7 +32,7 @@
>
>   #ifndef R_QUECOUNT
>   // 16, 32, 64, 128, 256, 512
>  -#define R_QUECOUNT 6
>  +#define R_QUECOUNT 512
>   #endif//R_QUECOUNT
>  
This slows down the pool considerably.

>  
>   #ifndef R_RZ
>  @@ -50,7 +50,7 @@
>
>   #ifndef R_STACK
>   // R_STACK is the number of stack entries to store in blocks for  
>  debug purposes
>  -#define R_STACK 6
>  +#define R_STACK 32
>   #else // R_STACK
>   #if R_STACK > 0 && R_STACK < 6
>   /* Increase the frame depth to get a reasonable back trace */
>  @@ -152,7 +152,7 @@
>   }
>   #endif//R_STACK 
This just makes debugging easier but uses 6x more memory per pool  
allocation and is much slower.

>  
>  -static int
>  +static rulong
>   RQueWhich ( rulong size )
>   {
>   	rulong que, quesize;
>  @@ -888,7 +888,7 @@
>   	rulong UsedSize;
>   	PR_FREE FreeBlock;
>   	rulong UserSize;
>  -	int que;
>  +	long que;
>
>   	ASSERT(pool);
>   	if ( !Addr ) 
What? Huh? How does this fix VFAT or do anything?

>   Index: base/system/winlogon/screensaver.c
>  ===================================================================
>  --- base/system/winlogon/screensaver.c	(revision 31587)
>  +++ base/system/winlogon/screensaver.c	(working copy)
>  @@ -3,7 +3,7 @@
>    * PROJECT:         ReactOS Winlogon
>    * FILE:            base/system/winlogon/screensaver.c
>    * PURPOSE:         Screen saver management
>  - * PROGRAMMERS:     Herv� Poussineau (hpoussin(a)reactos.org)
>  + * PROGRAMMERS:     Herv� Poussineau (hpoussin(a)reactos.org)
>    */
>
>   /* INCLUDES  
>  *****************************************************************/
>  @@ -43,20 +43,30 @@
>   	OUT LPDWORD Timeout)
>   {
>   	BOOL Enabled;
>  -
>  +	if(Timeout==NULL)
>  +	{
>  +		ERR("TimeOut is NULL!!!");
>  +		return;	
>  +	} 
Okay...but...this isn't an optional parameter, so it *should* crash  
if it's 0.

>    	if (!SystemParametersInfoW(SPI_GETSCREENSAVETIMEOUT,
> 0, Timeout, 0))
>   	{
>  -		WARN("WL: Unable to get screen saver timeout (error %lu).  
>  Disabling it\n", GetLastError());
>  -		*Timeout = INFINITE;
>  +		ERR("WL: Unable to get screen saver timeout (error %lu).  
>  Disabling it\n", GetLastError());
>  +		if(Timeout==NULL)
>  +		{
>  +			ERR("TimeOut is still NULL!!!");
>  +			return;	
>  +		} 
How can it be still NULL if you've already exited the function above?!


>   +		else
>  +			*Timeout = INFINITE;
>   	}
>   	else if (!SystemParametersInfoW(SPI_GETSCREENSAVEACTIVE, 0,  
>  &Enabled, 0))
>   	{
>  -		WARN("WL: Unable to check if screen saver is enabled (error % 
>  lu). Disabling it\n", GetLastError());
>  +		ERR("WL: Unable to check if screen saver is enabled (error %lu).  
>  Disabling it\n", GetLastError());
>   		*Timeout = INFINITE;
>   	}
>   	else if (!Enabled)
>   	{
>  -		TRACE("WL: Screen saver is disabled\n");
>  +		ERR("WL: Screen saver is disabled\n");
>   		*Timeout = INFINITE;
>   	}
>   	else
>  @@ -84,7 +94,7 @@
>   	if (!ImpersonateLoggedOnUser(Session->UserToken))
>   	{
>   		ERR("ImpersonateLoggedOnUser() failed with error %lu\n",  
>  GetLastError());
>  -		return 0;
>  +		goto cleanup;
>   	}
>
>   	Session->hUserActivity = CreateEventW(NULL, FALSE, FALSE, NULL); 
What do these useless screensaver debug print changes have to do with  
VFAT?

>   Index: base/system/winlogon/setup.c
>  ===================================================================
>  --- base/system/winlogon/setup.c	(revision 31587)
>  +++ base/system/winlogon/setup.c	(working copy)
>  @@ -19,13 +19,15 @@
>   DWORD
>   GetSetupType(VOID)
>   {
>  +	//CONTEXT("GetSetupType");
>  +	 
Huh?

>    	DWORD dwError;
>   	HKEY hKey;
>   	DWORD dwType;
>   	DWORD dwSize;
>   	DWORD dwSetupType;
>
>  -	TRACE("GetSetupType()\n");
>  +	ERR("GetSetupType()\n");
>
>   	/* Open key */
>   	dwError = RegOpenKeyExW(
>  @@ -48,11 +50,12 @@
>   		&dwSize);
>
>   	/* Close key, and check if returned values are correct */
>  -	RegCloseKey(hKey);
>  +	if(dwError==ERROR_SUCCESS)
>  +		RegCloseKey(hKey); 
You're leaking the key handle now!

>    	if (dwError != ERROR_SUCCESS || dwType != REG_DWORD
> || dwSize !=  
>  sizeof(DWORD))
>   		return 0;
>
>  -	TRACE("GetSetupType() returns %lu\n", dwSetupType);
>  +	ERR("GetSetupType() returns %lu\n", dwSetupType);
>   	return dwSetupType;
>   }
>
>  @@ -71,7 +74,7 @@
>   	DWORD dwSize;
>   	DWORD dwExitCode;
>
>  -	TRACE("RunSetup() called\n");
>  +	ERR("RunSetup() called\n");
>
>   	/* Open key */
>   	dwError = RegOpenKeyExW(
>  @@ -107,7 +110,7 @@
>   	else
>   		return FALSE;
>
>  -	TRACE("Should run '%s' now\n", debugstr_w(CommandLine));
>  +	ERR("Should run '%s' now\n", debugstr_w(CommandLine));
>
>   	/* Start process */
>   	StartupInfo.cb = sizeof(StartupInfo);
>  @@ -130,7 +133,7 @@
>   		&ProcessInformation);
>   	if (!Result)
>   	{
>  -		TRACE("Failed to run setup process\n");
>  +		ERR("Failed to run setup process\n");
>   		return FALSE;
>   	}
>
>  @@ -143,7 +146,7 @@
>   	CloseHandle(ProcessInformation.hThread);
>   	CloseHandle(ProcessInformation.hProcess);
>
>  -	TRACE ("RunSetup() done\n");
>  +	ERR ("RunSetup() done\n");
>
>   	return TRUE;
>   } 
How does spamming the debug log help VFAT?

>   Index: dll/win32/newdev/newdev.c
>  ===================================================================
>  --- dll/win32/newdev/newdev.c	(revision 31587)
>  +++ dll/win32/newdev/newdev.c	(working copy)
>  @@ -1,7 +1,7 @@
>   /*
>    * New device installer (newdev.dll)
>    *
>  - * Copyright 2005-2006 Herv� Poussineau (hpoussin(a)reactos.org)
>  + * Copyright 2005-2006 Herv� Poussineau (hpoussin(a)reactos.org)
>    *           2005 Christoph von Wittich (Christoph(a)ActiveVB.de)
>    *
>    * This library is free software; you can redistribute it and/or
>  @@ -718,7 +718,7 @@
>   	{
>   		TRACE("SetupDiGetDeviceRegistryProperty() failed with error 0x%x  
>  (InstanceId %s)\n",
>   			GetLastError(), debugstr_w(InstanceId));
>  -		goto cleanup;
>  +		//goto cleanup; 
Now this code won't return and who knows what side effects that  
causes...

>    	}
>
>   	if (SetupDiGetDeviceRegistryPropertyW(
>  Index: dll/win32/gdi32/objects/dc.c
>  ===================================================================
>  --- dll/win32/gdi32/objects/dc.c	(revision 31587)
>  +++ dll/win32/gdi32/objects/dc.c	(working copy)
>  @@ -58,12 +58,12 @@
>                         hspool,
>                        (PVOID) NULL,       // NULL for now.
>                        (PVOID) &UMdhpdev );
>  -#if 0
>  +#if 1
>   // Handle something other than a normal dc object.
>    if (GDI_HANDLE_GET_TYPE(hDC) != GDI_OBJECT_TYPE_DC)
>    {
>  -    PDC_ATTR Dc_Attr;
>  -    PLDC pLDC;
>  +    PDC_ATTR Dc_Attr=NULL;
>  +    PLDC pLDC=NULL;
>
>       GdiGetHandleUserData((HGDIOBJ) hDC, GDI_OBJECT_TYPE_DC,  
>  (PVOID) &Dc_Attr);
>
>  @@ -254,7 +254,7 @@
>   DeleteDC(HDC hDC)
>   {
>     BOOL Ret = TRUE;
>  -#if 0
>  +#if 1
>     PDC_ATTR Dc_Attr;
>     PLDC pLDC;
>
>  @@ -1342,9 +1342,9 @@
>   SelectObject(HDC hDC,
>                HGDIOBJ hGdiObj)
>   {
>  -    PDC_ATTR pDc_Attr;
>  -//    HGDIOBJ hOldObj = NULL;
>  -//    PTEB pTeb;
>  +    PDC_ATTR pDc_Attr=NULL;
>  +    HGDIOBJ hOldObj = NULL;
>  +    //PTEB pTeb=NULL;
>
>       if(!GdiGetHandleUserData(hDC, GDI_OBJECT_TYPE_DC, (PVOID) 
>  &pDc_Attr))
>       {
>  @@ -1355,9 +1355,10 @@
>       hGdiObj = GdiFixUpHandle(hGdiObj);
>       if (!GdiIsHandleValid(hGdiObj))
>       {
>  +    	DPRINT1("SelectObject:Handle not valid");
>           return NULL;
>       }
>  -
>  +	
>       UINT uType = GDI_HANDLE_GET_TYPE(hGdiObj);
>
>       switch (uType)
>  @@ -1369,28 +1370,39 @@
>               return NtGdiSelectBitmap(hDC, hGdiObj);
>
>           case GDI_OBJECT_TYPE_BRUSH:
>  -#if 0 // enable this when support is ready in win32k
>  -            hOldObj = pDc_Attr->hbrush;
>  -            pDc_Attr->ulDirty_ |= DC_BRUSH_DIRTY;
>  -            pDc_Attr->hbrush = hGdiObj;
>  +#if 1 // enable this when support is ready in win32k
>  +			if(pDc_Attr)
>  +			{
>  +				DPRINT1("SelectObject:Selectng brush & returning old brush");
>  +            	hOldObj = pDc_Attr->hbrush;
>  +            	pDc_Attr->ulDirty_ |= DC_BRUSH_DIRTY;
>  +            	pDc_Attr->hbrush = hGdiObj;
>  +			}
>  +			NtGdiSelectBrush(hDC, hGdiObj);
>               return hOldObj;
>   #endif
>  -            return NtGdiSelectBrush(hDC, hGdiObj);
>  +
>
>           case GDI_OBJECT_TYPE_PEN:
>   //        case GDI_OBJECT_TYPE_EXTPEN:
>  -#if 0 // enable this when support is ready in win32k
>  -            hOldObj = pDc_Attr->hpen;
>  -            pDc_Attr->ulDirty_ |= DC_PEN_DIRTY;
>  -            pDc_Attr->hpen = hGdiObj;
>  +#if 1 // enable this when support is ready in win32k
>  +			if(pDc_Attr)
>  +			{
>  +				DPRINT1("SelectObject:Selectng pen & returning old pen");
>  +            	hOldObj = pDc_Attr->hpen;
>  +            	pDc_Attr->ulDirty_ |= DC_PEN_DIRTY;
>  +            	pDc_Attr->hpen = hGdiObj;
>  +			}
>  +			NtGdiSelectPen(hDC, hGdiObj);
>               return hOldObj;
>   #endif
>  -            return NtGdiSelectPen(hDC, hGdiObj);
>  +
>
>           case GDI_OBJECT_TYPE_FONT:
>   #if 0
>               pTeb = NtCurrentTeb();
>  -            if (((pTeb->GdiTebBatch.HDC == 0) ||
>  +
>  +            if (pTeb!=NULL && ((pTeb->GdiTebBatch.HDC == 0) ||
>                    (pTeb->GdiTebBatch.HDC == (ULONG)hDC)) &&
>                   ((pTeb->GdiTebBatch.Offset + sizeof(GDIBSOBJECT))  
>  <= GDIBATCHBUFSIZE) &&
>                  (!(pDc_Attr->ulDirty_ & DC_DIBSECTION))) 
The TEB can't be NULL, and why did you re-enable James' disabled  
code? He disabled it for a reason. How is this related to VFAT?

>   Index: dll/win32/gdi32/misc/misc.c
>  ===================================================================
>  --- dll/win32/gdi32/misc/misc.c	(revision 31587)
>  +++ dll/win32/gdi32/misc/misc.c	(working copy)
>  @@ -129,18 +129,23 @@
>       // Need to test if we have Read & Write access to the VM  
>  address space.
>       //
>         BOOL Result = TRUE;
>  -      if(Entry->UserData)
>  +      if(Entry && Entry->UserData)
>         {
>            volatile CHAR *Current = (volatile CHAR*)Entry->UserData;
>  -         _SEH_TRY
>  +         if(Current==NULL)
>  +         	Result=FALSE; 
How can Current be NULL? You've already checked it's not NULL and  
you've got SEH below...

>   +         else
>            {
>  -           *Current = *Current;
>  +         	_SEH_TRY
>  +         	{
>  +           	*Current = *Current;
>  +         	}
>  +         	_SEH_HANDLE
>  +         	{
>  +           	Result = FALSE;
>  +         	}
>  +         	_SEH_END
>            }
>  -         _SEH_HANDLE
>  -         {
>  -           Result = FALSE;
>  -         }
>  -         _SEH_END
>         }
>          else
>            Result = FALSE; // Can not be zero.
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev 

This almost look like a robot-generated patch of completely random,  
useless and broken code changes. How is any of this useful to FireFox  
and VFAT?

Best regards,
Alex Ionescu

[Oriol]

Thankyou Alex for your comments, I modified some sources without knowing
what was i doing ..i was testing, i'll remove these useless patches.


Alex Ionescu wrote:
>   
>  On 3-Jan-08, at 4:41 PM, Oriol wrote:
>  
> >   I get an error while trying to install Firefox
> > 2.0 on a bootcd
> >  compilation, finally i found the possibly bug is in vfatfs.sys i
> >  modified some lines but is not my code.....it seems to install Firefox.
> >  Also, testing firefox i found problems with gdi and also done some
> >  modifications, still testing them..
> >  Here is my patch..NO WARRANTY he he..
> >
> >  Index: tools/rbuild/configuration.cpp
> >  ===================================================================
> >  --- tools/rbuild/configuration.cpp    (revision 31587)
> >  +++ tools/rbuild/configuration.cpp    (working copy)
> >  @@ -28,7 +28,7 @@
> >       CheckDependenciesForModuleOnly = false;
> >       CompilationUnitsEnabled = true;
> >       MakeHandlesInstallDirectories = false;
> >  -    GenerateProxyMakefilesInSourceTree = false;
> >  +    GenerateProxyMakefilesInSourceTree = true;
> >       InstallFiles = false;
> >       UseConfigurationInPath = false;
> >       UseVSVersionInPath = false; 
>  
>  What? Why? How does this help fix a VFAT bug?
>  
> >   Index: ntoskrnl/include/internal/mm.h
> >  ===================================================================
> >  --- ntoskrnl/include/internal/mm.h    (revision 31587)
> >  +++ ntoskrnl/include/internal/mm.h    (working copy)
> >  @@ -103,7 +103,7 @@
> >   /*
> >    * Maximum size of the kmalloc area (this is totally arbitary)
> >    */
> >  -#define MM_KERNEL_MAP_SIZE                  (16*1024*1024)
> >  +#define MM_KERNEL_MAP_SIZE                  (64*1024*1024)
> >   #define MM_KERNEL_MAP_BASE                  (0xf0c00000)
> >
> >   /* 
>  
>  What? Nobody uses this value anymore, and why would increasing it to
>  64MB help? These values need to go away.
>  
> >   Index: ntoskrnl/mm/rpoolmgr.h
> >  ===================================================================
> >  --- ntoskrnl/mm/rpoolmgr.h    (revision 31587)
> >  +++ ntoskrnl/mm/rpoolmgr.h    (working copy)
> >  @@ -32,7 +32,7 @@
> >
> >   #ifndef R_QUECOUNT
> >   // 16, 32, 64, 128, 256, 512
> >  -#define R_QUECOUNT 6
> >  +#define R_QUECOUNT 512
> >   #endif//R_QUECOUNT
> >  
>  
>  This slows down the pool considerably.
>  
> >  
> >   #ifndef R_RZ
> >  @@ -50,7 +50,7 @@
> >
> >   #ifndef R_STACK
> >   // R_STACK is the number of stack entries to store in blocks for
> >  debug purposes
> >  -#define R_STACK 6
> >  +#define R_STACK 32
> >   #else // R_STACK
> >   #if R_STACK > 0 && R_STACK < 6
> >   /* Increase the frame depth to get a reasonable back trace */
> >  @@ -152,7 +152,7 @@
> >   }
> >   #endif//R_STACK 
>  
>  This just makes debugging easier but uses 6x more memory per pool
>  allocation and is much slower.
>  
> >  
> >  -static int
> >  +static rulong
> >   RQueWhich ( rulong size )
> >   {
> >       rulong que, quesize;
> >  @@ -888,7 +888,7 @@
> >       rulong UsedSize;
> >       PR_FREE FreeBlock;
> >       rulong UserSize;
> >  -    int que;
> >  +    long que;
> >
> >       ASSERT(pool);
> >       if ( !Addr ) 
>  
>  What? Huh? How does this fix VFAT or do anything?
>  
> >   Index: base/system/winlogon/screensaver.c
> >  ===================================================================
> >  --- base/system/winlogon/screensaver.c    (revision 31587)
> >  +++ base/system/winlogon/screensaver.c    (working copy)
> >  @@ -3,7 +3,7 @@
> >    * PROJECT:         ReactOS Winlogon
> >    * FILE:            base/system/winlogon/screensaver.c
> >    * PURPOSE:         Screen saver management
> >  - * PROGRAMMERS:     Herv� Poussineau (hpoussin(a)reactos.org)
> >  + * PROGRAMMERS:     Herv� Poussineau (hpoussin(a)reactos.org)
> >    */
> >
> >   /* INCLUDES
> >  *****************************************************************/
> >  @@ -43,20 +43,30 @@
> >       OUT LPDWORD Timeout)
> >   {
> >       BOOL Enabled;
> >  -
> >  +    if(Timeout==NULL)
> >  +    {
> >  +        ERR("TimeOut is NULL!!!");
> >  +        return;   
> >  +    } 
>  
>  Okay...but...this isn't an optional parameter, so it *should* crash if
>  it's 0.
>  
> >        if
> > (!SystemParametersInfoW(SPI_GETSCREENSAVETIMEOUT, 0, Timeout, 0))
> >       {
> >  -        WARN("WL: Unable to get screen saver timeout (error %lu).
> >  Disabling it\n", GetLastError());
> >  -        *Timeout = INFINITE;
> >  +        ERR("WL: Unable to get screen saver timeout (error %lu).
> >  Disabling it\n", GetLastError());
> >  +        if(Timeout==NULL)
> >  +        {
> >  +            ERR("TimeOut is still NULL!!!");
> >  +            return;   
> >  +        } 
>  
>  How can it be still NULL if you've already exited the function above?!
>  
>  
> >   +        else
> >  +            *Timeout = INFINITE;
> >       }
> >       else if (!SystemParametersInfoW(SPI_GETSCREENSAVEACTIVE, 0,
> >  &Enabled, 0))
> >       {
> >  -        WARN("WL: Unable to check if screen saver is enabled (error
> >  %lu). Disabling it\n", GetLastError());
> >  +        ERR("WL: Unable to check if screen saver is enabled (error
> >  %lu). Disabling it\n", GetLastError());
> >           *Timeout = INFINITE;
> >       }
> >       else if (!Enabled)
> >       {
> >  -        TRACE("WL: Screen saver is disabled\n");
> >  +        ERR("WL: Screen saver is disabled\n");
> >           *Timeout = INFINITE;
> >       }
> >       else
> >  @@ -84,7 +94,7 @@
> >       if (!ImpersonateLoggedOnUser(Session->UserToken))
> >       {
> >           ERR("ImpersonateLoggedOnUser() failed with error %lu\n",
> >  GetLastError());
> >  -        return 0;
> >  +        goto cleanup;
> >       }
> >
> >       Session->hUserActivity = CreateEventW(NULL, FALSE, FALSE, NULL); 
>  
>  What do these useless screensaver debug print changes have to do with VFAT?
>  
> >   Index: base/system/winlogon/setup.c
> >  ===================================================================
> >  --- base/system/winlogon/setup.c    (revision 31587)
> >  +++ base/system/winlogon/setup.c    (working copy)
> >  @@ -19,13 +19,15 @@
> >   DWORD
> >   GetSetupType(VOID)
> >   {
> >  +    //CONTEXT("GetSetupType");
> >  +    
>  
>  Huh?
>  
> >        DWORD dwError;
> >       HKEY hKey;
> >       DWORD dwType;
> >       DWORD dwSize;
> >       DWORD dwSetupType;
> >
> >  -    TRACE("GetSetupType()\n");
> >  +    ERR("GetSetupType()\n");
> >
> >       /* Open key */
> >       dwError = RegOpenKeyExW(
> >  @@ -48,11 +50,12 @@
> >           &dwSize);
> >
> >       /* Close key, and check if returned values are correct */
> >  -    RegCloseKey(hKey);
> >  +    if(dwError==ERROR_SUCCESS)
> >  +        RegCloseKey(hKey); 
>  
>  You're leaking the key handle now!
>  
> >        if (dwError != ERROR_SUCCESS || dwType !=
> > REG_DWORD || dwSize !=
> >  sizeof(DWORD))
> >           return 0;
> >
> >  -    TRACE("GetSetupType() returns %lu\n", dwSetupType);
> >  +    ERR("GetSetupType() returns %lu\n", dwSetupType);
> >       return dwSetupType;
> >   }
> >
> >  @@ -71,7 +74,7 @@
> >       DWORD dwSize;
> >       DWORD dwExitCode;
> >
> >  -    TRACE("RunSetup() called\n");
> >  +    ERR("RunSetup() called\n");
> >
> >       /* Open key */
> >       dwError = RegOpenKeyExW(
> >  @@ -107,7 +110,7 @@
> >       else
> >           return FALSE;
> >
> >  -    TRACE("Should run '%s' now\n", debugstr_w(CommandLine));
> >  +    ERR("Should run '%s' now\n", debugstr_w(CommandLine));
> >
> >       /* Start process */
> >       StartupInfo.cb = sizeof(StartupInfo);
> >  @@ -130,7 +133,7 @@
> >           &ProcessInformation);
> >       if (!Result)
> >       {
> >  -        TRACE("Failed to run setup process\n");
> >  +        ERR("Failed to run setup process\n");
> >           return FALSE;
> >       }
> >
> >  @@ -143,7 +146,7 @@
> >       CloseHandle(ProcessInformation.hThread);
> >       CloseHandle(ProcessInformation.hProcess);
> >
> >  -    TRACE ("RunSetup() done\n");
> >  +    ERR ("RunSetup() done\n");
> >
> >       return TRUE;
> >   } 
>  
>  How does spamming the debug log help VFAT?
>  
> >   Index: dll/win32/newdev/newdev.c
> >  ===================================================================
> >  --- dll/win32/newdev/newdev.c    (revision 31587)
> >  +++ dll/win32/newdev/newdev.c    (working copy)
> >  @@ -1,7 +1,7 @@
> >   /*
> >    * New device installer (newdev.dll)
> >    *
> >  - * Copyright 2005-2006 Herv� Poussineau (hpoussin(a)reactos.org)
> >  + * Copyright 2005-2006 Herv� Poussineau (hpoussin(a)reactos.org)
> >    *           2005 Christoph von Wittich (Christoph(a)ActiveVB.de)
> >    *
> >    * This library is free software; you can redistribute it and/or
> >  @@ -718,7 +718,7 @@
> >       {
> >           TRACE("SetupDiGetDeviceRegistryProperty() failed with error
> >  0x%x (InstanceId %s)\n",
> >               GetLastError(), debugstr_w(InstanceId));
> >  -        goto cleanup;
> >  +        //goto cleanup; 
>  
>  Now this code won't return and who knows what side effects that causes...
>  
> >        }
> >
> >       if (SetupDiGetDeviceRegistryPropertyW(
> >  Index: dll/win32/gdi32/objects/dc.c
> >  ===================================================================
> >  --- dll/win32/gdi32/objects/dc.c    (revision 31587)
> >  +++ dll/win32/gdi32/objects/dc.c    (working copy)
> >  @@ -58,12 +58,12 @@
> >                         hspool,
> >                        (PVOID) NULL,       // NULL for now.
> >                        (PVOID) &UMdhpdev );
> >  -#if 0
> >  +#if 1
> >   // Handle something other than a normal dc object.
> >    if (GDI_HANDLE_GET_TYPE(hDC) != GDI_OBJECT_TYPE_DC)
> >    {
> >  -    PDC_ATTR Dc_Attr;
> >  -    PLDC pLDC;
> >  +    PDC_ATTR Dc_Attr=NULL;
> >  +    PLDC pLDC=NULL;
> >
> >       GdiGetHandleUserData((HGDIOBJ) hDC, GDI_OBJECT_TYPE_DC, (PVOID)
> >  &Dc_Attr);
> >
> >  @@ -254,7 +254,7 @@
> >   DeleteDC(HDC hDC)
> >   {
> >     BOOL Ret = TRUE;
> >  -#if 0
> >  +#if 1
> >     PDC_ATTR Dc_Attr;
> >     PLDC pLDC;
> >
> >  @@ -1342,9 +1342,9 @@
> >   SelectObject(HDC hDC,
> >                HGDIOBJ hGdiObj)
> >   {
> >  -    PDC_ATTR pDc_Attr;
> >  -//    HGDIOBJ hOldObj = NULL;
> >  -//    PTEB pTeb;
> >  +    PDC_ATTR pDc_Attr=NULL;
> >  +    HGDIOBJ hOldObj = NULL;
> >  +    //PTEB pTeb=NULL;
> >
> >       if(!GdiGetHandleUserData(hDC, GDI_OBJECT_TYPE_DC, (PVOID)&pDc_Attr))
> >       {
> >  @@ -1355,9 +1355,10 @@
> >       hGdiObj = GdiFixUpHandle(hGdiObj);
> >       if (!GdiIsHandleValid(hGdiObj))
> >       {
> >  +        DPRINT1("SelectObject:Handle not valid");
> >           return NULL;
> >       }
> >  -
> >  +   
> >       UINT uType = GDI_HANDLE_GET_TYPE(hGdiObj);
> >
> >       switch (uType)
> >  @@ -1369,28 +1370,39 @@
> >               return NtGdiSelectBitmap(hDC, hGdiObj);
> >
> >           case GDI_OBJECT_TYPE_BRUSH:
> >  -#if 0 // enable this when support is ready in win32k
> >  -            hOldObj = pDc_Attr->hbrush;
> >  -            pDc_Attr->ulDirty_ |= DC_BRUSH_DIRTY;
> >  -            pDc_Attr->hbrush = hGdiObj;
> >  +#if 1 // enable this when support is ready in win32k
> >  +            if(pDc_Attr)
> >  +            {
> >  +                DPRINT1("SelectObject:Selectng brush & returning old
> >  brush");
> >  +                hOldObj = pDc_Attr->hbrush;
> >  +                pDc_Attr->ulDirty_ |= DC_BRUSH_DIRTY;
> >  +                pDc_Attr->hbrush = hGdiObj;
> >  +            }
> >  +            NtGdiSelectBrush(hDC, hGdiObj);
> >               return hOldObj;
> >   #endif
> >  -            return NtGdiSelectBrush(hDC, hGdiObj);
> >  +
> >
> >           case GDI_OBJECT_TYPE_PEN:
> >   //        case GDI_OBJECT_TYPE_EXTPEN:
> >  -#if 0 // enable this when support is ready in win32k
> >  -            hOldObj = pDc_Attr->hpen;
> >  -            pDc_Attr->ulDirty_ |= DC_PEN_DIRTY;
> >  -            pDc_Attr->hpen = hGdiObj;
> >  +#if 1 // enable this when support is ready in win32k
> >  +            if(pDc_Attr)
> >  +            {
> >  +                DPRINT1("SelectObject:Selectng pen & returning old
> >  pen");
> >  +                hOldObj = pDc_Attr->hpen;
> >  +                pDc_Attr->ulDirty_ |= DC_PEN_DIRTY;
> >  +                pDc_Attr->hpen = hGdiObj;
> >  +            }
> >  +            NtGdiSelectPen(hDC, hGdiObj);
> >               return hOldObj;
> >   #endif
> >  -            return NtGdiSelectPen(hDC, hGdiObj);
> >  +
> >
> >           case GDI_OBJECT_TYPE_FONT:
> >   #if 0
> >               pTeb = NtCurrentTeb();
> >  -            if (((pTeb->GdiTebBatch.HDC == 0) ||
> >  +
> >  +            if (pTeb!=NULL && ((pTeb->GdiTebBatch.HDC == 0) ||
> >                    (pTeb->GdiTebBatch.HDC == (ULONG)hDC)) &&
> >                   ((pTeb->GdiTebBatch.Offset + sizeof(GDIBSOBJECT)) <=
> >  GDIBATCHBUFSIZE) &&
> >                  (!(pDc_Attr->ulDirty_ & DC_DIBSECTION))) 
>  
>  The TEB can't be NULL, and why did you re-enable James' disabled code?
>  He disabled it for a reason. How is this related to VFAT?
>  
> >   Index: dll/win32/gdi32/misc/misc.c
> >  ===================================================================
> >  --- dll/win32/gdi32/misc/misc.c    (revision 31587)
> >  +++ dll/win32/gdi32/misc/misc.c    (working copy)
> >  @@ -129,18 +129,23 @@
> >       // Need to test if we have Read & Write access to the VM address
> >  space.
> >       //
> >         BOOL Result = TRUE;
> >  -      if(Entry->UserData)
> >  +      if(Entry && Entry->UserData)
> >         {
> >            volatile CHAR *Current = (volatile CHAR*)Entry->UserData;
> >  -         _SEH_TRY
> >  +         if(Current==NULL)
> >  +             Result=FALSE; 
>  
>  How can Current be NULL? You've already checked it's not NULL and you've
>  got SEH below...
>  
> >   +         else
> >            {
> >  -           *Current = *Current;
> >  +             _SEH_TRY
> >  +             {
> >  +               *Current = *Current;
> >  +             }
> >  +             _SEH_HANDLE
> >  +             {
> >  +               Result = FALSE;
> >  +             }
> >  +             _SEH_END
> >            }
> >  -         _SEH_HANDLE
> >  -         {
> >  -           Result = FALSE;
> >  -         }
> >  -         _SEH_END
> >         }
> >          else
> >            Result = FALSE; // Can not be zero.
> >  _______________________________________________
> >  Ros-dev mailing list
> >  Ros-dev(a)reactos.org
> >  http://www.reactos.org/mailman/listinfo/ros-dev 
>  
>  
>  This almost look like a robot-generated patch of completely random,
>  useless and broken code changes. How is any of this useful to FireFox
>  and VFAT?
>  
>  Best regards,
>  Alex Ionescu
>  
>  
>  
>  ------------------------------------------------------------------------
>  
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev 


		
______________________________________________ 
LLama Gratis a cualquier PC del Mundo. 
Llamadas a fijos y móviles desde 1 céntimo por minuto. 
http://es.voice.yahoo.com