On 2014-11-11 23:16:21, Tian, Feng wrote: > 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.
I wonder what Olivier is supposed to do with this patch. I notice it lacks a Contributed-under and Signed-off-by for your changes. I assume you'll add that, and then Olivier will add it to the commit messages? Another (I think easier) approach would be to provide feedback and let Olivier modify his code based on your suggestions. You might also give your r-b and let Olivier commit the patch series since he has commit access. (It seems he had a 3 patch series originally.) -Jordan ------------------------------------------------------------------------------ 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
