[ros-dev] [ros-diffs] [hbelusca] 58971: [FREELDR] - Fix dprints (be careful when "Status" variables are booleans). - Don't fail when trying to load an non-existent KD transport dll.

Timo Kreuzer timo.kreuzer at web.de
Thu May 9 13:23:14 UTC 2013


I suggest NEVER to use Status as the name for anything other than 
NTSTATUS variables.
Otherwise problems are almost preprogrammed (seen this multiple times). 
It Should be called Result or something.

This is one of those cases where hungarian notation really makes a 
difference, Colin. ;-)



Am 08.05.2013 19:52, schrieb hbelusca at svn.reactos.org:
> Author: hbelusca
> Date: Wed May  8 17:52:16 2013
> New Revision: 58971
>
> URL: http://svn.reactos.org/svn/reactos?rev=58971&view=rev
> Log:
> [FREELDR]
> - Fix dprints (be careful when "Status" variables are booleans).
> - Don't fail when trying to load an non-existent KD transport dll.
>
> Modified:
>      branches/kd++/boot/freeldr/freeldr/disk/scsiport.c
>      branches/kd++/boot/freeldr/freeldr/windows/peloader.c
>      branches/kd++/boot/freeldr/freeldr/windows/winldr.c
>
> Modified: branches/kd++/boot/freeldr/freeldr/disk/scsiport.c
> URL: http://svn.reactos.org/svn/reactos/branches/kd%2B%2B/boot/freeldr/freeldr/disk/scsiport.c?rev=58971&r1=58970&r2=58971&view=diff
> ==============================================================================
> --- branches/kd++/boot/freeldr/freeldr/disk/scsiport.c	[iso-8859-1] (original)
> +++ branches/kd++/boot/freeldr/freeldr/disk/scsiport.c	[iso-8859-1] Wed May  8 17:52:16 2013
> @@ -1568,7 +1568,7 @@
>       ULONG ImportTableSize;
>       PLDR_DATA_TABLE_ENTRY BootDdDTE, FreeldrDTE;
>       CHAR NtBootDdPath[MAX_PATH];
> -    PVOID ImageBase;
> +    PVOID ImageBase = NULL;
>       ULONG (NTAPI *EntryPoint)(IN PVOID DriverObject, IN PVOID RegistryPath);
>       BOOLEAN Status;
>   
>
> Modified: branches/kd++/boot/freeldr/freeldr/windows/peloader.c
> URL: http://svn.reactos.org/svn/reactos/branches/kd%2B%2B/boot/freeldr/freeldr/windows/peloader.c?rev=58971&r1=58970&r2=58971&view=diff
> ==============================================================================
> --- branches/kd++/boot/freeldr/freeldr/windows/peloader.c	[iso-8859-1] (original)
> +++ branches/kd++/boot/freeldr/freeldr/windows/peloader.c	[iso-8859-1] Wed May  8 17:52:16 2013
> @@ -255,11 +255,12 @@
>   	return TRUE;
>   }
>   
> -/* WinLdrLoadImage loads the specified image from the file (it doesn't
> -   perform any additional operations on the filename, just directly
> -   calls the file I/O routines), and relocates it so that it's ready
> -   to be used when paging is enabled.
> -   Addressing mode: physical
> +/*
> + * WinLdrLoadImage loads the specified image from the file (it doesn't
> + * perform any additional operations on the filename, just directly
> + * calls the file I/O routines), and relocates it so that it's ready
> + * to be used when paging is enabled.
> + * Addressing mode: physical
>    */
>   BOOLEAN
>   WinLdrLoadImage(IN PCHAR FileName,
> @@ -429,7 +430,6 @@
>   	/* If loading failed - return right now */
>   	if (Status != ESUCCESS)
>   		return FALSE;
> -
>   
>   	/* Relocate the image, if it needs it */
>   	if (NtHeaders->OptionalHeader.ImageBase != (ULONG_PTR)VirtualBase)
> @@ -756,7 +756,7 @@
>   {
>   	CHAR FullDllName[256];
>   	BOOLEAN Status;
> -	PVOID BasePA;
> +	PVOID BasePA = NULL;
>   
>   	/* Prepare the full path to the file to be loaded */
>   	strcpy(FullDllName, DirectoryPath);
> @@ -781,7 +781,7 @@
>   		DataTableEntry);
>   	if (!Status)
>   	{
> -		ERR("WinLdrAllocateDataTableEntry() failed with Status=0x%X\n", Status);
> +		ERR("WinLdrAllocateDataTableEntry() failed\n");
>   		return Status;
>   	}
>   
> @@ -791,7 +791,7 @@
>   	Status = WinLdrScanImportDescriptorTable(ModuleListHead, DirectoryPath, *DataTableEntry);
>   	if (!Status)
>   	{
> -		ERR("WinLdrScanImportDescriptorTable() failed with Status=0x%X\n", Status);
> +		ERR("WinLdrScanImportDescriptorTable() failed\n");
>   		return Status;
>   	}
>   
>
> Modified: branches/kd++/boot/freeldr/freeldr/windows/winldr.c
> URL: http://svn.reactos.org/svn/reactos/branches/kd%2B%2B/boot/freeldr/freeldr/windows/winldr.c?rev=58971&r1=58970&r2=58971&view=diff
> ==============================================================================
> --- branches/kd++/boot/freeldr/freeldr/windows/winldr.c	[iso-8859-1] (original)
> +++ branches/kd++/boot/freeldr/freeldr/windows/winldr.c	[iso-8859-1] Wed May  8 17:52:16 2013
> @@ -237,7 +237,7 @@
>   	CHAR DllName[1024];
>   	PCHAR DriverNamePos;
>   	BOOLEAN Status;
> -	PVOID DriverBase;
> +	PVOID DriverBase = NULL;
>   
>   	// Separate the path to file name and directory path
>   	_snprintf(DriverPath, sizeof(DriverPath), "%wZ", FilePath);
> @@ -258,7 +258,6 @@
>   	}
>   
>   	TRACE("DriverPath: %s, DllName: %s, LPB\n", DriverPath, DllName);
> -
>   
>   	// Check if driver is already loaded
>   	Status = WinLdrCheckForLoadedDll(LoadOrderListHead, DllName, DriverDTE);
> @@ -424,7 +423,7 @@
>   }
>   
>   static
> -PVOID
> +BOOLEAN
>   LoadModule(
>       PLOADER_PARAMETER_BLOCK LoaderBlock,
>       PCCH Path,
> @@ -434,10 +433,10 @@
>       BOOLEAN IsKdTransportDll,
>       ULONG Percentage)
>   {
> +	BOOLEAN Status;
>   	CHAR FullFileName[MAX_PATH];
>   	CHAR ProgressString[256];
> -	NTSTATUS Status;
> -	PVOID BaseAdress;
> +	PVOID BaseAdress = NULL;
>   
>   	UiDrawBackdrop();
>   	sprintf(ProgressString, "Loading %s...", File);
> @@ -448,8 +447,12 @@
>   	strcat(FullFileName, File);
>   
>   	Status = WinLdrLoadImage(FullFileName, MemoryType, &BaseAdress);
> -	TRACE("%s loaded with status %d at %p\n",
> -	        File, Status, BaseAdress);
> +	if (!Status)
> +	{
> +		TRACE("Loading %s failed\n", File);
> +		return FALSE;
> +	}
> +	TRACE("%s loaded successfully at %p\n", File, BaseAdress);
>   
>   	strcpy(FullFileName, "WINDOWS\\SYSTEM32\\");
>   	strcat(FullFileName, File);
> @@ -458,13 +461,13 @@
>   	 * the Kernel Debugger Transport DLL, to make the
>   	 * PE loader happy.
>   	 */
> -	WinLdrAllocateDataTableEntry(&LoaderBlock->LoadOrderListHead,
> -	                             (IsKdTransportDll ? "KDCOM.DLL" : File),
> -	                             FullFileName,
> -	                             BaseAdress,
> -	                             Dte);
> -
> -	return BaseAdress;
> +	Status = WinLdrAllocateDataTableEntry(&LoaderBlock->LoadOrderListHead,
> +	                                      (IsKdTransportDll ? "KDCOM.DLL" : File),
> +	                                      FullFileName,
> +	                                      BaseAdress,
> +	                                      Dte);
> +
> +	return Status;
>   }
>   
>   static
> @@ -679,11 +682,11 @@
>   	UiDrawBackdrop();
>   	UiDrawProgressBarCenter(15, 100, "Loading system hive...");
>   	Status = WinLdrInitSystemHive(LoaderBlock, BootPath);
> -	TRACE("SYSTEM hive loaded with status %d\n", Status);
> +	TRACE("SYSTEM hive %s\n", (Status ? "loaded" : "not loaded"));
>   
>   	/* Load NLS data, OEM font, and prepare boot drivers list */
>   	Status = WinLdrScanSystemHive(LoaderBlock, BootPath);
> -	TRACE("SYSTEM hive scanned with status %d\n", Status);
> +	TRACE("SYSTEM hive %s\n", (Status ? "scanned" : "not scanned"));
>   
>   	/* Finish loading */
>   	LoadAndBootWindowsCommon(OperatingSystemVersion,
>
>
>




More information about the Ros-dev mailing list