Thanks a lot for submitted a revised version.
There is nothing controversial in your changes. Except maybe these lines in
FvSimpleFileSystemGetInfo():
Size = sizeof (EFI_FILE_SYSTEM_INFO) + StrSize (Instance->VolumeLabel) -
sizeof (CHAR16);
CopyMem (FsInfoOut, &mFsInfoTemplate, Size);
While it was in the original version:
CopyMem (FsInfoOut, &mFsInfoTemplate, sizeof(EFI_FILE_SYSTEM_INFO));
With mFsInfoTemplate:
EFI_FILE_SYSTEM_INFO mFsInfoTemplate = {
0, // Populate at runtime
TRUE, // Read-only
0, // Don't know volume size
0, // No free space
0, // Don't know block size
L"" // Populate at runtime
};
It means you are copying more bytes than the actual source variable.
> -----Original Message-----
> From: Tian, Feng [mailto:feng.t...@intel.com]
> Sent: 12 November 2014 07:16
> To: Olivier Martin
> Cc: edk2-devel@lists.sourceforge.net; Tian, Feng
> Subject: [edk2] patch review feedback about FvSimpleFileSystem module
>
> Hi, Martin
>
> Here is my review comments on your FvSimpleFileSystem uefi driver.
> Sorry for late response.
>
> 1. Fixed system hang due to wrong assertions in
> FvSimpleFileSystemDriverStart() and FvSimpleFileSystemOpenVolume().
> a. change ASSERT (NumChars == GUID_STRING_SIZE); to ASSERT
> ((NumChars + 1) * sizeof (CHAR16) == GUID_STRING_SIZE);
> b. change ASSERT (NumChars == FVFS_VOLUME_LABEL_SIZE); to
> ASSERT ((NumChars + 1) * sizeof (CHAR16) == FVFS_VOLUME_LABEL_SIZE);
> 2. Fixed SimpleFileSystem SCT errors.
> a. EFI_FILE_PROTOCOL.GetInfo() should handle VolumeLabel info
> rather than return EFI_UNSUPPORTED.
> b. EFI_FILE_PROTOCOL.GetInfo() should initial Size field of
> EFI_FILE_SYSTEM_INFO at run time.
> c. Some interfaces in EFI_FILE_PROTOCOL should return
> EFI_WRITE_PROTECTED rather than EFI_UNSUPPORTED.
> d. EFI_FILE_PROTOCOL.Delete() should return
> EFI_WARN_DELETE_FAILURE rather than EFI_UNSUPPORTED.
> 3. Added missing ComponentName protocol interface and modify code to
> use APIs defined in UefiLib to install DriverBinding and ComponentName
> protocol.
> 4. Fixed wrong UnicodeCollation protocol usage.
> 5. Added missing function comments.
> 6. Added missing user extension meta data in INF file.
> 7. Remove BasePathLib dependency and use internal implementation as
> the move operation for this library and others in ShellPkg need more
> discussions.
>
> I attached the modified path for your review, will check it in to
> MdeModulePkg/Universal after pass your review. BasePathLib will be on
> other thread.
>
> Thanks
> Feng
------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://pubads.g.doubleclick.net/gampad/clk?id=154624111&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
