[ros-diffs] [ion] 69452: [BOOTLIB]: - Fix multiple bugs in ETFS code (confusion between file offset vs. disk offset) - Implement EtfsGetInformation, EtfsSetInformation, and fix ETFS_FILE definition to make thi...

Author: ion
Date: Mon Oct  5 05:53:40 2015
New Revision: 69452

URL: http://svn.reactos.org/svn/reactos?rev=69452&view=rev
Log:
[BOOTLIB]:
- Fix multiple bugs in ETFS code (confusion between file offset vs. disk offset)
- Implement EtfsGetInformation, EtfsSetInformation, and fix ETFS_FILE definition to make
this easy.
- Implement EtfsRead.
- Fix multiple bugs in file I/O code (swapped/reversed validation checks)
- Make BlStatusPrint call EfiPrintf on debug builds, even without BD.
- Add some additional error logging.

Modified:
    trunk/reactos/boot/environ/app/bootmgr/bootmgr.h
    trunk/reactos/boot/environ/include/bl.h
    trunk/reactos/boot/environ/lib/io/etfs.c
    trunk/reactos/boot/environ/lib/io/file.c
    trunk/reactos/boot/environ/lib/misc/debug.c
    trunk/reactos/boot/environ/lib/misc/image.c

Modified: trunk/reactos/boot/environ/app/bootmgr/bootmgr.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/environ/app/bootmgr/b…
==============================================================================
--- trunk/reactos/boot/environ/app/bootmgr/bootmgr.h	[iso-8859-1] (original)
+++ trunk/reactos/boot/environ/app/bootmgr/bootmgr.h	[iso-8859-1] Mon Oct  5 05:53:40
2015
@@ -49,7 +49,7 @@
     ULONG Size;
 } BL_PACKED_BOOT_ERROR, *PBL_PACKED_BOOT_ERROR;
 
-#define BL_FATAL_ERROR_BCD_READ 0x02
+#define BL_FATAL_ERROR_BCD_READ 0x01
 
 /* FUNCTIONS *****************************************************************/
 

Modified: trunk/reactos/boot/environ/include/bl.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/environ/include/bl.h?…
==============================================================================
--- trunk/reactos/boot/environ/include/bl.h	[iso-8859-1] (original)
+++ trunk/reactos/boot/environ/include/bl.h	[iso-8859-1] Mon Oct  5 05:53:40 2015
@@ -321,7 +321,7 @@
     _In_ struct _BL_FILE_ENTRY* FileEntry,
     _In_ PVOID Buffer,
     _In_ ULONG Size,
-    _Out_ PULONG BytesRead
+    _Out_opt_ PULONG BytesRead
     );
 
 typedef
@@ -786,8 +786,10 @@
 
 typedef struct _BL_FILE_INFORMATION
 {
-    ULONGLONG FileSize;
-    ULONGLONG CurrentOffset;
+    ULONGLONG Size;
+    ULONGLONG Offset;
+    PWCHAR FsName;
+    ULONG Flags;
 } BL_FILE_INFORMATION, *PBL_FILE_INFORMATION;
 
 typedef struct _BL_FILE_CALLBACKS
@@ -809,7 +811,7 @@
     ULONG Flags;
     ULONG ReferenceCount;
     ULONG Unknown;
-    ULONGLONG Unknown1;
+    ULONGLONG TotalBytesRead;
     ULONGLONG Unknown2;
     BL_FILE_CALLBACKS Callbacks;
     PVOID FsSpecificData;

Modified: trunk/reactos/boot/environ/lib/io/etfs.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/environ/lib/io/etfs.c…
==============================================================================
--- trunk/reactos/boot/environ/lib/io/etfs.c	[iso-8859-1] (original)
+++ trunk/reactos/boot/environ/lib/io/etfs.c	[iso-8859-1] Mon Oct  5 05:53:40 2015
@@ -36,12 +36,12 @@
 
 typedef struct _BL_ETFS_FILE
 {
+    ULONG DiskOffset;
     ULONG DirOffset;
     ULONG DirEntOffset;
-    ULONGLONG Size;
-    ULONGLONG Offset;
-    PWCHAR FsName;
-    ULONG Flags;
+
+    BL_FILE_INFORMATION;
+
     ULONG DeviceId;
 } BL_ETFS_FILE, *PBL_ETFS_FILE;
 
@@ -56,9 +56,35 @@
     _Out_ PBL_FILE_ENTRY *FileEntry
     );
 
+NTSTATUS
+EtfsGetInformation (
+    _In_ PBL_FILE_ENTRY FileEntry,
+    _Out_ PBL_FILE_INFORMATION FileInfo
+    );
+
+NTSTATUS
+EtfsSetInformation (
+    _In_ PBL_FILE_ENTRY FileEntry,
+    _In_ PBL_FILE_INFORMATION FileInfo
+    );
+
+NTSTATUS
+EtfsRead (
+    _In_ PBL_FILE_ENTRY FileEntry,
+    _In_ PVOID Buffer,
+    _In_ ULONG Size,
+    _Out_opt_ PULONG BytesReturned
+    );
+
 BL_FILE_CALLBACKS EtfsFunctionTable =
 {
     EtfsOpen,
+    NULL,
+    EtfsRead,
+    NULL,
+    NULL,
+    EtfsGetInformation,
+    EtfsSetInformation
 };
 
 /* FUNCTIONS *****************************************************************/
@@ -205,7 +231,7 @@
 
     EtfsFile = DirectoryEntry->FsSpecificData;
     DeviceId = EtfsFile->DeviceId;
-    FileOffset = EtfsFile->Offset;
+    FileOffset = EtfsFile->DiskOffset;
     EtfsDevice = EtfsDeviceTable[DeviceId];
 
     DirectoryOffset = *DirentOffset;
@@ -362,7 +388,7 @@
     DirentOffset = EtfsFile->DirEntOffset;
 
     if ((KeepOffset) ||
-        (ALIGN_DOWN_BY((DirentOffset + EtfsFile->Offset), CD_SECTOR_SIZE) ==
+        (ALIGN_DOWN_BY((DirentOffset + EtfsFile->DiskOffset), CD_SECTOR_SIZE) ==
          EtfsDevice->Offset))
     {
         Status = EtfspGetDirent(DirectoryEntry, &Dirent, &DirentOffset);
@@ -404,6 +430,108 @@
     }
 
     return Status;
+}
+
+NTSTATUS
+EtfsRead (
+    _In_ PBL_FILE_ENTRY FileEntry,
+    _In_ PVOID Buffer, 
+    _In_ ULONG Size,
+    _Out_opt_ PULONG BytesReturned
+    )
+{
+    ULONG BytesRead;
+    PBL_ETFS_FILE EtfsFile;
+    NTSTATUS Status;
+
+    /* Assume failure for now */
+    BytesRead = 0;
+
+    /* Make sure that the read is within the file's boundaries */
+    EtfsFile = FileEntry->FsSpecificData;
+    if ((Size + EtfsFile->Offset) > EtfsFile->Size)
+    {
+        /* Bail out otherwise */
+        Status = STATUS_INVALID_PARAMETER;
+    }
+    else
+    {
+        /* Read the offset that matches this file's offset, on the disk */
+        Status = BlDeviceReadAtOffset(FileEntry->DeviceId,
+                                      Size,
+                                      EtfsFile->Offset + EtfsFile->DiskOffset,
+                                      Buffer,
+                                      &BytesRead);
+        if (NT_SUCCESS(Status))
+        {
+            /* Update the file offset and return the size as having been read */
+            EtfsFile->Offset += Size;
+            BytesRead = Size;
+        }
+    }
+
+    /* Check if caller wanted to know how many bytes were read */
+    if (BytesReturned)
+    {
+        /* Return the value */
+        *BytesReturned = BytesRead;
+    }
+
+    /* All done */
+    return Status;
+}
+
+NTSTATUS
+EtfsSetInformation (
+    _In_ PBL_FILE_ENTRY FileEntry,
+    _In_ PBL_FILE_INFORMATION FileInfo
+    )
+{
+    PBL_ETFS_FILE EtfsFile;
+    BL_FILE_INFORMATION LocalFileInfo;
+
+    /* Get the underlying ETFS file data structure */
+    EtfsFile = (PBL_ETFS_FILE)FileEntry->FsSpecificData;
+
+    /* Make a copy of the incoming attributes, but ignore the new offset */
+    LocalFileInfo = *FileInfo;
+    LocalFileInfo.Offset = EtfsFile->Offset;
+
+    /* Check if these match exactly the current file */
+    if (!RtlEqualMemory(&LocalFileInfo, &EtfsFile->Size, sizeof(*FileInfo)))
+    {
+        /* Nope -- which means caller is trying to change an immutable */
+        EfiPrintf(L"Incorrect information change\r\n");
+        return STATUS_INVALID_PARAMETER;
+    }
+
+    /* Is the offset past the end of the file? */
+    if (FileInfo->Offset >= EtfsFile->Size)
+    {
+        /* Don't allow EOF */
+        EfiPrintf(L"Offset too large: %lx vs %lx \r\n", FileInfo->Offset,
EtfsFile->Size);
+        return STATUS_INVALID_PARAMETER;
+    }
+
+    /* Update the offset */
+    EtfsFile->Offset = FileInfo->Offset;
+    return STATUS_SUCCESS;
+}
+
+NTSTATUS
+EtfsGetInformation (
+    _In_ PBL_FILE_ENTRY FileEntry,
+    _Out_ PBL_FILE_INFORMATION FileInfo
+    )
+{
+    PBL_ETFS_FILE EtfsFile;
+
+    /* Get the underlying ETFS file data structure */
+    EtfsFile = (PBL_ETFS_FILE)FileEntry->FsSpecificData;
+
+    /* Copy the cached information structure within it */
+    RtlCopyMemory(FileInfo, &EtfsFile->Size, sizeof(*FileInfo));
+    return STATUS_SUCCESS;
 }
 
 NTSTATUS
@@ -486,7 +614,7 @@
     RtlCopyMemory(&NewFile->Callbacks,
                   &EtfsFunctionTable,
                   sizeof(NewFile->Callbacks));
-    EtfsFile->Offset = FileOffset;
+    EtfsFile->DiskOffset = FileOffset;
     EtfsFile->DirOffset = DirOffset;
     EtfsFile->Size = FileSize;
     EtfsFile->DeviceId = DeviceId;
@@ -794,7 +922,7 @@
     RootEntry->FsSpecificData = EtfsFile;
     EtfsFile->DeviceId = DeviceId;
     EtfsFile->Flags |= 1;
-    EtfsFile->Offset = EtfsDevice->RootDirOffset;
+    EtfsFile->DiskOffset = EtfsDevice->RootDirOffset;
     EtfsFile->DirOffset = 0;
     EtfsFile->Size = EtfsDevice->RootDirSize;
     EtfsFile->FsName = L"cdfs";

Modified: trunk/reactos/boot/environ/lib/io/file.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/environ/lib/io/file.c…
==============================================================================
--- trunk/reactos/boot/environ/lib/io/file.c	[iso-8859-1] (original)
+++ trunk/reactos/boot/environ/lib/io/file.c	[iso-8859-1] Mon Oct  5 05:53:40 2015
@@ -434,7 +434,7 @@
     if (++FileEntry->ReferenceCount == 1)
     {
         /* Reset unknowns */
-        FileEntry->Unknown1 = 0;
+        FileEntry->TotalBytesRead = 0;
         FileEntry->Unknown2 = 0;
     }
 
@@ -548,7 +548,7 @@
     }
 
     /* Validate file ID */
-    if (FileEntries > FileId)
+    if (FileId > FileEntries)
     {
         return STATUS_INVALID_PARAMETER;
     }
@@ -579,7 +579,7 @@
     }
 
     /* Validate file ID */
-    if (FileEntries > FileId)
+    if (FileId > FileEntries)
     {
         return STATUS_INVALID_PARAMETER;
     }
@@ -612,7 +612,7 @@
     Size = 0;
 
     /* Make sure we didn't overshoot */
-    if (FileInformation->CurrentOffset > FileInformation->FileSize)
+    if (FileInformation->Offset > FileInformation->Size)
     {
         /* Bail out */
         Status = STATUS_INVALID_PARAMETER;
@@ -621,9 +621,9 @@
 
     /* Compute the appropriate 32-bit size of this read, based on file size */
     Size = ULONG_MAX;
-    if ((FileInformation->FileSize - FileInformation->CurrentOffset) <=
ULONG_MAX)
-    {
-        Size = (ULONG)(FileInformation->FileSize) -
(ULONG)(FileInformation->CurrentOffset);
+    if ((FileInformation->Size - FileInformation->Offset) <= ULONG_MAX)
+    {
+        Size = (ULONG)(FileInformation->Size) - (ULONG)(FileInformation->Offset);
     }
 
     /* Check if the caller has an input buffer */
@@ -683,7 +683,7 @@
     }
 
     /* Bail out of the file ID is invalid */
-    if (FileEntries > FileId)
+    if (FileId > FileEntries)
     {
         return STATUS_INVALID_PARAMETER;
     }
@@ -774,7 +774,7 @@
     }
 
     /* Increment the number of bytes read */
-    FileEntry->Unknown1 += RequiredSize;
+    FileEntry->TotalBytesRead += RequiredSize;
 
     /* Check if the unknown flag on the device was changed during this routine */
     if (ChangedUnknown)
@@ -811,8 +811,8 @@
     }
 
     /* Save the current offset, and overwrite it with the one we want */
-    FileOffset = FileInfo.CurrentOffset;
-    FileInfo.CurrentOffset = ByteOffset;
+    FileOffset = FileInfo.Offset;
+    FileInfo.Offset = ByteOffset;
 
     /* Check the validity of the read and the actual size to read */
     RequiredSize = Size;
@@ -824,11 +824,12 @@
     if (!NT_SUCCESS(Status))
     {
         /* Bail out if the read is invalid */
+        EfiPrintf(L"File info check failure: %lx\n", Status);
         return Status;
     }
 
     /* Check if the offset we're requesting is not the current offset */
-    if (FileInfo.CurrentOffset != FileOffset)
+    if (FileInfo.Offset != FileOffset)
     {
         /* Set the new offset to use */
         Status = BlFileSetInformation(FileId, &FileInfo);
@@ -848,10 +849,10 @@
     if (!NT_SUCCESS(Status))
     {
         /* The read failed -- had we modified the offset? */
-        if (FileInfo.CurrentOffset != FileOffset)
+        if (FileInfo.Offset != FileOffset)
         {
             /* Restore the offset back to its original value */
-            FileInfo.CurrentOffset = FileOffset;
+            FileInfo.Offset = FileOffset;
             BlFileSetInformation(FileId, &FileInfo);
         }
     }

Modified: trunk/reactos/boot/environ/lib/misc/debug.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/environ/lib/misc/debu…
==============================================================================
--- trunk/reactos/boot/environ/lib/misc/debug.c	[iso-8859-1] (original)
+++ trunk/reactos/boot/environ/lib/misc/debug.c	[iso-8859-1] Mon Oct  5 05:53:40 2015
@@ -83,11 +83,19 @@
     va_start(va, Format);
 
     /* Check if the boot debugger is enabled */
-    if (BlBdDebuggerEnabled())
+    if (BlBdDebuggerEnabled()
+#if (defined(DBG))
+        || TRUE
+#endif
+    )
     {
         /* Print the string out into a buffer */
         if (vswprintf(BlScratchBuffer, Format, va) > 0)
         {
+#if defined(DBG)
+            EfiPrintf(BlScratchBuffer);
+            EfiPrintf(L"\r\n");
+#endif
             /* Make it a UNICODE_STRING */
             RtlInitUnicodeString(&UnicodeString, BlScratchBuffer);
 

Modified: trunk/reactos/boot/environ/lib/misc/image.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/boot/environ/lib/misc/imag…
==============================================================================
--- trunk/reactos/boot/environ/lib/misc/image.c	[iso-8859-1] (original)
+++ trunk/reactos/boot/environ/lib/misc/image.c	[iso-8859-1] Mon Oct  5 05:53:40 2015
@@ -39,8 +39,8 @@
         }
 
         /* We only support files less than 4GB in the Image Mapped */
-        Size = FileInformation.FileSize;
-        if (FileInformation.FileSize > ULONG_MAX)
+        Size = FileInformation.Size;
+        if (FileInformation.Size > ULONG_MAX)
         {
             return STATUS_NOT_SUPPORTED;
         }
@@ -392,6 +392,7 @@
     Status = ImgpOpenFile(DeviceId, FileName, DeviceId, &FileHandle);
     if (!NT_SUCCESS(Status))
     {
+        EfiPrintf(L"Error opening file: %lx\r\n", Status);
         goto Quickie;
     }
 
@@ -399,6 +400,7 @@
     Status = ImgpGetFileSize(&FileHandle, &ImageSize);
     if (!NT_SUCCESS(Status))
     {
+        EfiPrintf(L"Error getting file size: %lx\r\n", Status);
         goto Quickie;
     }