With an empty string isn't StrSize going to return 2, and then sizeof(CHAR16) also returning 2, so the math will result in the same copy as the original?
-Jaben > -----Original Message----- > From: Olivier Martin [mailto:olivier.mar...@arm.com] > Sent: Wednesday, November 12, 2014 3:35 AM > To: Tian, Feng > Cc: edk2-devel@lists.sourceforge.net > Subject: Re: [edk2] patch review feedback about FvSimpleFileSystem > module > > 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.cl > ktrk > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ 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
