[edk2] [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

[Olivier Martin]

FvSimpleFileSystem adds '.efi' to the EFI application and drivers
filenames even through this extension is not present in the real
filename of the EFI module.

In the current behaviour, it would not be possible to open an EFI
application using FvSimpleFileSystem if the extension has been omitted
in the given filename.
It can be create some confusion if someone wants to try to
open a file with the real application name (eg: 'Shell').

This patch adds support to try again to look for the file with the
extension if it had failed to find it without the extension.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Olivier Martin <***@arm.com>
---
.../FvSimpleFileSystemDxe/FvSimpleFileSystem.c | 62 +++++++++++++++++-----
1 file changed, 50 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
index b0e7dc3..c6137ac 100644
--- a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
+++ b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
@@ -483,6 +483,10 @@ FvSimpleFileSystemOpen (
FV_FILESYSTEM_FILE *NewFile;
FV_FILESYSTEM_FILE_INFO *FvFileInfo;
LIST_ENTRY *FvFileInfoLink;
+ EFI_STATUS Status;
+ UINTN FileNameLength;
+ UINTN NewFileNameLength;
+ CHAR16 *FileNameWithExtension;

//
// Check for a valid mode
@@ -531,26 +535,60 @@ FvSimpleFileSystemOpen (
//
// Do a linear search for a file in the FV with a matching filename
//
+ Status = EFI_NOT_FOUND;
+ FvFileInfo = NULL;
for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
!IsNull (&Instance->FileInfoHead, FvFileInfoLink);
FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileName) == 0) {
- NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
- if (NewFile == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }

- NewFile->Signature = FVFS_FILE_SIGNATURE;
- NewFile->Instance = Instance;
- NewFile->FvFileInfo = FvFileInfo;
- CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
- InitializeListHead (&NewFile->Link);
- InsertHeadList (&Instance->FileHead, &NewFile->Link);
+ // If the file has not been found check if the filename exists with an extension
+ // in case there was no extension present.
+ // FvFileSystem adds a 'virtual' extension '.EFI' to EFI applications and drivers
+ // present in the Firmware Volume
+ if (Status == EFI_NOT_FOUND) {
+ FileNameLength = StrLen (FileName);
+
+ // Does the filename already contain the '.EFI' extension?
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, FileName + FileNameLength - 4, L".efi") != 0) {
+ // No, there was no extension. So add one and search again for the file
+ // NewFileNameLength = FileNameLength + 1 + 4 = (Number of non-null character) + (file extension) + (a null character)
+ NewFileNameLength = FileNameLength + 1 + 4;
+ FileNameWithExtension = AllocateCopyPool (NewFileNameLength * 2, FileName);
+ StrCatS (FileNameWithExtension, NewFileNameLength, L".EFI");
+
+ for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
+ !IsNull (&Instance->FileInfoHead, FvFileInfoLink);
+ FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
+ FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileNameWithExtension) == 0) {
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }
+ }
+ }

- *NewHandle = &NewFile->FileProtocol;
- return EFI_SUCCESS;
+ if (!EFI_ERROR (Status)) {
+ NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
+ if (NewFile == NULL) {
+ return EFI_OUT_OF_RESOURCES;
}
+
+ NewFile->Signature = FVFS_FILE_SIGNATURE;
+ NewFile->Instance = Instance;
+ NewFile->FvFileInfo = FvFileInfo;
+ CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
+ InitializeListHead (&NewFile->Link);
+ InsertHeadList (&Instance->FileHead, &NewFile->Link);
+
+ *NewHandle = &NewFile->FileProtocol;
+ return EFI_SUCCESS;
}

return EFI_NOT_FOUND;

[Tian, Feng]

Hi, Martin

I am a little confused by your description on the issue.

EDKII build system puts $BASE_NAME string to UI section and generate $BASE_NAME.efi file name at build directory. The FvSimpleFileSystem driver adds .efi postfix to string gotten from each UI section for all executable file types. So my understanding is these two strings are identical and should not bring confusion.

And why it's not possible to open an EFI application without the .efi extension? If user enter "Shell" or "Shell.efi", it should run for both cases.

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Tuesday, July 07, 2015 00:17
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net; Olivier Martin; Olivier Martin
Subject: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

FvSimpleFileSystem adds '.efi' to the EFI application and drivers filenames even through this extension is not present in the real filename of the EFI module.

In the current behaviour, it would not be possible to open an EFI application using FvSimpleFileSystem if the extension has been omitted in the given filename.
It can be create some confusion if someone wants to try to open a file with the real application name (eg: 'Shell').

This patch adds support to try again to look for the file with the extension if it had failed to find it without the extension.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Olivier Martin <***@arm.com>
---
.../FvSimpleFileSystemDxe/FvSimpleFileSystem.c | 62 +++++++++++++++++-----
1 file changed, 50 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
index b0e7dc3..c6137ac 100644
--- a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
+++ b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
@@ -483,6 +483,10 @@ FvSimpleFileSystemOpen (
FV_FILESYSTEM_FILE *NewFile;
FV_FILESYSTEM_FILE_INFO *FvFileInfo;
LIST_ENTRY *FvFileInfoLink;
+ EFI_STATUS Status;
+ UINTN FileNameLength;
+ UINTN NewFileNameLength;
+ CHAR16 *FileNameWithExtension;

//
// Check for a valid mode
@@ -531,26 +535,60 @@ FvSimpleFileSystemOpen (
//
// Do a linear search for a file in the FV with a matching filename
//
+ Status = EFI_NOT_FOUND;
+ FvFileInfo = NULL;
for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
!IsNull (&Instance->FileInfoHead, FvFileInfoLink);
FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileName) == 0) {
- NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
- if (NewFile == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }

- NewFile->Signature = FVFS_FILE_SIGNATURE;
- NewFile->Instance = Instance;
- NewFile->FvFileInfo = FvFileInfo;
- CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
- InitializeListHead (&NewFile->Link);
- InsertHeadList (&Instance->FileHead, &NewFile->Link);
+ // If the file has not been found check if the filename exists with
+ an extension // in case there was no extension present.
+ // FvFileSystem adds a 'virtual' extension '.EFI' to EFI applications
+ and drivers // present in the Firmware Volume if (Status ==
+ EFI_NOT_FOUND) {
+ FileNameLength = StrLen (FileName);
+
+ // Does the filename already contain the '.EFI' extension?
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, FileName + FileNameLength - 4, L".efi") != 0) {
+ // No, there was no extension. So add one and search again for the file
+ // NewFileNameLength = FileNameLength + 1 + 4 = (Number of non-null character) + (file extension) + (a null character)
+ NewFileNameLength = FileNameLength + 1 + 4;
+ FileNameWithExtension = AllocateCopyPool (NewFileNameLength * 2, FileName);
+ StrCatS (FileNameWithExtension, NewFileNameLength, L".EFI");
+
+ for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
+ !IsNull (&Instance->FileInfoHead, FvFileInfoLink);
+ FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
+ FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileNameWithExtension) == 0) {
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }
+ }
+ }

- *NewHandle = &NewFile->FileProtocol;
- return EFI_SUCCESS;
+ if (!EFI_ERROR (Status)) {
+ NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
+ if (NewFile == NULL) {
+ return EFI_OUT_OF_RESOURCES;
}
+
+ NewFile->Signature = FVFS_FILE_SIGNATURE;
+ NewFile->Instance = Instance;
+ NewFile->FvFileInfo = FvFileInfo;
+ CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
+ InitializeListHead (&NewFile->Link);
+ InsertHeadList (&Instance->FileHead, &NewFile->Link);
+
+ *NewHandle = &NewFile->FileProtocol;
+ return EFI_SUCCESS;
}

return EFI_NOT_FOUND;
--
2.1.1

[Olivier Martin]

You are saying "If user enter "Shell" or "Shell.efi", it should run for both cases."
But it is actually the case I am trying to solve with this patch :-)

FvSimpleFileSystemOpen() was only checking if there was strict equality between the given name and the name exposed by FvSimpleFileSystemDxe (ie: the name with .EFI).
It means if you try to open the file "Shell" (the name you gave in the UI section) it would not work.

-----Original Message-----
From: Tian, Feng [mailto:***@intel.com]
Sent: 08 July 2015 06:38
To: Olivier Martin
Cc: edk2-***@lists.sourceforge.net; Tian, Feng
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Hi, Martin

I am a little confused by your description on the issue.

EDKII build system puts $BASE_NAME string to UI section and generate $BASE_NAME.efi file name at build directory. The FvSimpleFileSystem driver adds .efi postfix to string gotten from each UI section for all executable file types. So my understanding is these two strings are identical and should not bring confusion.

And why it's not possible to open an EFI application without the .efi extension? If user enter "Shell" or "Shell.efi", it should run for both cases.

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Tuesday, July 07, 2015 00:17
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net; Olivier Martin; Olivier Martin
Subject: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

FvSimpleFileSystem adds '.efi' to the EFI application and drivers filenames even through this extension is not present in the real filename of the EFI module.

In the current behaviour, it would not be possible to open an EFI application using FvSimpleFileSystem if the extension has been omitted in the given filename.
It can be create some confusion if someone wants to try to open a file with the real application name (eg: 'Shell').

This patch adds support to try again to look for the file with the extension if it had failed to find it without the extension.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Olivier Martin <***@arm.com>
---
.../FvSimpleFileSystemDxe/FvSimpleFileSystem.c | 62 +++++++++++++++++-----
1 file changed, 50 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
index b0e7dc3..c6137ac 100644
--- a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
+++ b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
@@ -483,6 +483,10 @@ FvSimpleFileSystemOpen (
FV_FILESYSTEM_FILE *NewFile;
FV_FILESYSTEM_FILE_INFO *FvFileInfo;
LIST_ENTRY *FvFileInfoLink;
+ EFI_STATUS Status;
+ UINTN FileNameLength;
+ UINTN NewFileNameLength;
+ CHAR16 *FileNameWithExtension;

//
// Check for a valid mode
@@ -531,26 +535,60 @@ FvSimpleFileSystemOpen (
//
// Do a linear search for a file in the FV with a matching filename
//
+ Status = EFI_NOT_FOUND;
+ FvFileInfo = NULL;
for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
!IsNull (&Instance->FileInfoHead, FvFileInfoLink);
FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileName) == 0) {
- NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
- if (NewFile == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }

- NewFile->Signature = FVFS_FILE_SIGNATURE;
- NewFile->Instance = Instance;
- NewFile->FvFileInfo = FvFileInfo;
- CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
- InitializeListHead (&NewFile->Link);
- InsertHeadList (&Instance->FileHead, &NewFile->Link);
+ // If the file has not been found check if the filename exists with
+ an extension // in case there was no extension present.
+ // FvFileSystem adds a 'virtual' extension '.EFI' to EFI applications
+ and drivers // present in the Firmware Volume if (Status ==
+ EFI_NOT_FOUND) {
+ FileNameLength = StrLen (FileName);
+
+ // Does the filename already contain the '.EFI' extension?
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, FileName + FileNameLength - 4, L".efi") != 0) {
+ // No, there was no extension. So add one and search again for the file
+ // NewFileNameLength = FileNameLength + 1 + 4 = (Number of non-null character) + (file extension) + (a null character)
+ NewFileNameLength = FileNameLength + 1 + 4;
+ FileNameWithExtension = AllocateCopyPool (NewFileNameLength * 2, FileName);
+ StrCatS (FileNameWithExtension, NewFileNameLength, L".EFI");
+
+ for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
+ !IsNull (&Instance->FileInfoHead, FvFileInfoLink);
+ FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
+ FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileNameWithExtension) == 0) {
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }
+ }
+ }

- *NewHandle = &NewFile->FileProtocol;
- return EFI_SUCCESS;
+ if (!EFI_ERROR (Status)) {
+ NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
+ if (NewFile == NULL) {
+ return EFI_OUT_OF_RESOURCES;
}
+
+ NewFile->Signature = FVFS_FILE_SIGNATURE;
+ NewFile->Instance = Instance;
+ NewFile->FvFileInfo = FvFileInfo;
+ CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
+ InitializeListHead (&NewFile->Link);
+ InsertHeadList (&Instance->FileHead, &NewFile->Link);
+
+ *NewHandle = &NewFile->FileProtocol;
+ return EFI_SUCCESS;
}

return EFI_NOT_FOUND;
--
2.1.1


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782

[Tian, Feng]

Hi, Martin

I tested old code behavior on NT32 platform by adding MdeModulePkg/Application/VariableInfo/VariableInfo.inf & MdeModulePkg\Universal\FvSimpleFileSystemDxe\FvSimpleFileSystemDxe.inf to DSC&FDF files.

Under shell, the application VariableInfo could run by enter "VariableInfo" or "VariableInfo.efi" cmd.

Do I misunderstand something?

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Thursday, July 09, 2015 00:32
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

You are saying "If user enter "Shell" or "Shell.efi", it should run for both cases."
But it is actually the case I am trying to solve with this patch :-)

FvSimpleFileSystemOpen() was only checking if there was strict equality between the given name and the name exposed by FvSimpleFileSystemDxe (ie: the name with .EFI).
It means if you try to open the file "Shell" (the name you gave in the UI section) it would not work.

-----Original Message-----
From: Tian, Feng [mailto:***@intel.com]
Sent: 08 July 2015 06:38
To: Olivier Martin
Cc: edk2-***@lists.sourceforge.net; Tian, Feng
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Hi, Martin

I am a little confused by your description on the issue.

EDKII build system puts $BASE_NAME string to UI section and generate $BASE_NAME.efi file name at build directory. The FvSimpleFileSystem driver adds .efi postfix to string gotten from each UI section for all executable file types. So my understanding is these two strings are identical and should not bring confusion.

And why it's not possible to open an EFI application without the .efi extension? If user enter "Shell" or "Shell.efi", it should run for both cases.

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Tuesday, July 07, 2015 00:17
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net; Olivier Martin; Olivier Martin
Subject: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

FvSimpleFileSystem adds '.efi' to the EFI application and drivers filenames even through this extension is not present in the real filename of the EFI module.

In the current behaviour, it would not be possible to open an EFI application using FvSimpleFileSystem if the extension has been omitted in the given filename.
It can be create some confusion if someone wants to try to open a file with the real application name (eg: 'Shell').

This patch adds support to try again to look for the file with the extension if it had failed to find it without the extension.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Olivier Martin <***@arm.com>
---
.../FvSimpleFileSystemDxe/FvSimpleFileSystem.c | 62 +++++++++++++++++-----
1 file changed, 50 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
index b0e7dc3..c6137ac 100644
--- a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
+++ b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
@@ -483,6 +483,10 @@ FvSimpleFileSystemOpen (
FV_FILESYSTEM_FILE *NewFile;
FV_FILESYSTEM_FILE_INFO *FvFileInfo;
LIST_ENTRY *FvFileInfoLink;
+ EFI_STATUS Status;
+ UINTN FileNameLength;
+ UINTN NewFileNameLength;
+ CHAR16 *FileNameWithExtension;

//
// Check for a valid mode
@@ -531,26 +535,60 @@ FvSimpleFileSystemOpen (
//
// Do a linear search for a file in the FV with a matching filename
//
+ Status = EFI_NOT_FOUND;
+ FvFileInfo = NULL;
for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
!IsNull (&Instance->FileInfoHead, FvFileInfoLink);
FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileName) == 0) {
- NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
- if (NewFile == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }

- NewFile->Signature = FVFS_FILE_SIGNATURE;
- NewFile->Instance = Instance;
- NewFile->FvFileInfo = FvFileInfo;
- CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
- InitializeListHead (&NewFile->Link);
- InsertHeadList (&Instance->FileHead, &NewFile->Link);
+ // If the file has not been found check if the filename exists with
+ an extension // in case there was no extension present.
+ // FvFileSystem adds a 'virtual' extension '.EFI' to EFI applications
+ and drivers // present in the Firmware Volume if (Status ==
+ EFI_NOT_FOUND) {
+ FileNameLength = StrLen (FileName);
+
+ // Does the filename already contain the '.EFI' extension?
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, FileName + FileNameLength - 4, L".efi") != 0) {
+ // No, there was no extension. So add one and search again for the file
+ // NewFileNameLength = FileNameLength + 1 + 4 = (Number of non-null character) + (file extension) + (a null character)
+ NewFileNameLength = FileNameLength + 1 + 4;
+ FileNameWithExtension = AllocateCopyPool (NewFileNameLength * 2, FileName);
+ StrCatS (FileNameWithExtension, NewFileNameLength, L".EFI");
+
+ for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
+ !IsNull (&Instance->FileInfoHead, FvFileInfoLink);
+ FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
+ FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileNameWithExtension) == 0) {
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }
+ }
+ }

- *NewHandle = &NewFile->FileProtocol;
- return EFI_SUCCESS;
+ if (!EFI_ERROR (Status)) {
+ NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
+ if (NewFile == NULL) {
+ return EFI_OUT_OF_RESOURCES;
}
+
+ NewFile->Signature = FVFS_FILE_SIGNATURE;
+ NewFile->Instance = Instance;
+ NewFile->FvFileInfo = FvFileInfo;
+ CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
+ InitializeListHead (&NewFile->Link);
+ InsertHeadList (&Instance->FileHead, &NewFile->Link);
+
+ *NewHandle = &NewFile->FileProtocol;
+ return EFI_SUCCESS;
}

return EFI_NOT_FOUND;
--
2.1.1


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782

[Olivier Martin]

Can you try without using EFI Shell?
If you check at the code of FvSimpleFileSystemOpen(), there is no way it works.
I am guessing EFI Shell try to open the file with and without '.EFI' extension.

And actually, after searching quickly in ShellPkg, you can find:

/**
Find a file by searching the CWD and then the path with a variable set of file
extensions. If the file is not found it will append each extension in the list
in the order provided and return the first one that is successful.

If FileName is NULL, then ASSERT.
If FileExtension is NULL, then behavior is identical to ShellFindFilePath.

If the return value is not NULL then the memory must be caller freed.

@param[in] FileName Filename string.
@param[in] FileExtension Semi-colon delimeted list of possible extensions.

@retval NULL The file was not found.
@retval !NULL The path to the file.
**/
CHAR16 *
EFIAPI
ShellFindFilePathEx (
IN CONST CHAR16 *FileName,
IN CONST CHAR16 *FileExtension
)

-----Original Message-----
From: Tian, Feng [mailto:***@intel.com]
Sent: 09 July 2015 09:39
To: Olivier Martin
Cc: edk2-***@lists.sourceforge.net; Tian, Feng
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Hi, Martin

I tested old code behavior on NT32 platform by adding MdeModulePkg/Application/VariableInfo/VariableInfo.inf & MdeModulePkg\Universal\FvSimpleFileSystemDxe\FvSimpleFileSystemDxe.inf to DSC&FDF files.

Under shell, the application VariableInfo could run by enter "VariableInfo" or "VariableInfo.efi" cmd.

Do I misunderstand something?

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Thursday, July 09, 2015 00:32
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

You are saying "If user enter "Shell" or "Shell.efi", it should run for both cases."
But it is actually the case I am trying to solve with this patch :-)

FvSimpleFileSystemOpen() was only checking if there was strict equality between the given name and the name exposed by FvSimpleFileSystemDxe (ie: the name with .EFI).
It means if you try to open the file "Shell" (the name you gave in the UI section) it would not work.

-----Original Message-----
From: Tian, Feng [mailto:***@intel.com]
Sent: 08 July 2015 06:38
To: Olivier Martin
Cc: edk2-***@lists.sourceforge.net; Tian, Feng
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Hi, Martin

I am a little confused by your description on the issue.

EDKII build system puts $BASE_NAME string to UI section and generate $BASE_NAME.efi file name at build directory. The FvSimpleFileSystem driver adds .efi postfix to string gotten from each UI section for all executable file types. So my understanding is these two strings are identical and should not bring confusion.

And why it's not possible to open an EFI application without the .efi extension? If user enter "Shell" or "Shell.efi", it should run for both cases.

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Tuesday, July 07, 2015 00:17
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net; Olivier Martin; Olivier Martin
Subject: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

FvSimpleFileSystem adds '.efi' to the EFI application and drivers filenames even through this extension is not present in the real filename of the EFI module.

In the current behaviour, it would not be possible to open an EFI application using FvSimpleFileSystem if the extension has been omitted in the given filename.
It can be create some confusion if someone wants to try to open a file with the real application name (eg: 'Shell').

This patch adds support to try again to look for the file with the extension if it had failed to find it without the extension.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Olivier Martin <***@arm.com>
---
.../FvSimpleFileSystemDxe/FvSimpleFileSystem.c | 62 +++++++++++++++++-----
1 file changed, 50 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
index b0e7dc3..c6137ac 100644
--- a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
+++ b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
@@ -483,6 +483,10 @@ FvSimpleFileSystemOpen (
FV_FILESYSTEM_FILE *NewFile;
FV_FILESYSTEM_FILE_INFO *FvFileInfo;
LIST_ENTRY *FvFileInfoLink;
+ EFI_STATUS Status;
+ UINTN FileNameLength;
+ UINTN NewFileNameLength;
+ CHAR16 *FileNameWithExtension;

//
// Check for a valid mode
@@ -531,26 +535,60 @@ FvSimpleFileSystemOpen (
//
// Do a linear search for a file in the FV with a matching filename
//
+ Status = EFI_NOT_FOUND;
+ FvFileInfo = NULL;
for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
!IsNull (&Instance->FileInfoHead, FvFileInfoLink);
FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileName) == 0) {
- NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
- if (NewFile == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }

- NewFile->Signature = FVFS_FILE_SIGNATURE;
- NewFile->Instance = Instance;
- NewFile->FvFileInfo = FvFileInfo;
- CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
- InitializeListHead (&NewFile->Link);
- InsertHeadList (&Instance->FileHead, &NewFile->Link);
+ // If the file has not been found check if the filename exists with
+ an extension // in case there was no extension present.
+ // FvFileSystem adds a 'virtual' extension '.EFI' to EFI applications
+ and drivers // present in the Firmware Volume if (Status ==
+ EFI_NOT_FOUND) {
+ FileNameLength = StrLen (FileName);
+
+ // Does the filename already contain the '.EFI' extension?
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, FileName + FileNameLength - 4, L".efi") != 0) {
+ // No, there was no extension. So add one and search again for the file
+ // NewFileNameLength = FileNameLength + 1 + 4 = (Number of non-null character) + (file extension) + (a null character)
+ NewFileNameLength = FileNameLength + 1 + 4;
+ FileNameWithExtension = AllocateCopyPool (NewFileNameLength * 2, FileName);
+ StrCatS (FileNameWithExtension, NewFileNameLength, L".EFI");
+
+ for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
+ !IsNull (&Instance->FileInfoHead, FvFileInfoLink);
+ FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
+ FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileNameWithExtension) == 0) {
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }
+ }
+ }

- *NewHandle = &NewFile->FileProtocol;
- return EFI_SUCCESS;
+ if (!EFI_ERROR (Status)) {
+ NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
+ if (NewFile == NULL) {
+ return EFI_OUT_OF_RESOURCES;
}
+
+ NewFile->Signature = FVFS_FILE_SIGNATURE;
+ NewFile->Instance = Instance;
+ NewFile->FvFileInfo = FvFileInfo;
+ CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
+ InitializeListHead (&NewFile->Link);
+ InsertHeadList (&Instance->FileHead, &NewFile->Link);
+
+ *NewHandle = &NewFile->FileProtocol;
+ return EFI_SUCCESS;
}

return EFI_NOT_FOUND;
--
2.1.1


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782

[Tian, Feng]

Martin,

Got your point now.

Do you think such behavior (extension matching) should be done in caller or callee? From ShellPkg implementation, it's done by caller.

But if you vote the callee to do this, I am also ok with your patch:). Reveiwed-by: Feng Tian <***@intel.com>

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Thursday, July 09, 2015 16:53
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Can you try without using EFI Shell?
If you check at the code of FvSimpleFileSystemOpen(), there is no way it works.
I am guessing EFI Shell try to open the file with and without '.EFI' extension.

And actually, after searching quickly in ShellPkg, you can find:

/**
Find a file by searching the CWD and then the path with a variable set of file
extensions. If the file is not found it will append each extension in the list
in the order provided and return the first one that is successful.

If FileName is NULL, then ASSERT.
If FileExtension is NULL, then behavior is identical to ShellFindFilePath.

If the return value is not NULL then the memory must be caller freed.

@param[in] FileName Filename string.
@param[in] FileExtension Semi-colon delimeted list of possible extensions.

@retval NULL The file was not found.
@retval !NULL The path to the file.
**/
CHAR16 *
EFIAPI
ShellFindFilePathEx (
IN CONST CHAR16 *FileName,
IN CONST CHAR16 *FileExtension
)

-----Original Message-----
From: Tian, Feng [mailto:***@intel.com]
Sent: 09 July 2015 09:39
To: Olivier Martin
Cc: edk2-***@lists.sourceforge.net; Tian, Feng
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Hi, Martin

I tested old code behavior on NT32 platform by adding MdeModulePkg/Application/VariableInfo/VariableInfo.inf & MdeModulePkg\Universal\FvSimpleFileSystemDxe\FvSimpleFileSystemDxe.inf to DSC&FDF files.

Under shell, the application VariableInfo could run by enter "VariableInfo" or "VariableInfo.efi" cmd.

Do I misunderstand something?

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Thursday, July 09, 2015 00:32
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

You are saying "If user enter "Shell" or "Shell.efi", it should run for both cases."
But it is actually the case I am trying to solve with this patch :-)

FvSimpleFileSystemOpen() was only checking if there was strict equality between the given name and the name exposed by FvSimpleFileSystemDxe (ie: the name with .EFI).
It means if you try to open the file "Shell" (the name you gave in the UI section) it would not work.

-----Original Message-----
From: Tian, Feng [mailto:***@intel.com]
Sent: 08 July 2015 06:38
To: Olivier Martin
Cc: edk2-***@lists.sourceforge.net; Tian, Feng
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Hi, Martin

I am a little confused by your description on the issue.

EDKII build system puts $BASE_NAME string to UI section and generate $BASE_NAME.efi file name at build directory. The FvSimpleFileSystem driver adds .efi postfix to string gotten from each UI section for all executable file types. So my understanding is these two strings are identical and should not bring confusion.

And why it's not possible to open an EFI application without the .efi extension? If user enter "Shell" or "Shell.efi", it should run for both cases.

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Tuesday, July 07, 2015 00:17
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net; Olivier Martin; Olivier Martin
Subject: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

FvSimpleFileSystem adds '.efi' to the EFI application and drivers filenames even through this extension is not present in the real filename of the EFI module.

In the current behaviour, it would not be possible to open an EFI application using FvSimpleFileSystem if the extension has been omitted in the given filename.
It can be create some confusion if someone wants to try to open a file with the real application name (eg: 'Shell').

This patch adds support to try again to look for the file with the extension if it had failed to find it without the extension.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Olivier Martin <***@arm.com>
---
.../FvSimpleFileSystemDxe/FvSimpleFileSystem.c | 62 +++++++++++++++++-----
1 file changed, 50 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
index b0e7dc3..c6137ac 100644
--- a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
+++ b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
@@ -483,6 +483,10 @@ FvSimpleFileSystemOpen (
FV_FILESYSTEM_FILE *NewFile;
FV_FILESYSTEM_FILE_INFO *FvFileInfo;
LIST_ENTRY *FvFileInfoLink;
+ EFI_STATUS Status;
+ UINTN FileNameLength;
+ UINTN NewFileNameLength;
+ CHAR16 *FileNameWithExtension;

//
// Check for a valid mode
@@ -531,26 +535,60 @@ FvSimpleFileSystemOpen (
//
// Do a linear search for a file in the FV with a matching filename
//
+ Status = EFI_NOT_FOUND;
+ FvFileInfo = NULL;
for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
!IsNull (&Instance->FileInfoHead, FvFileInfoLink);
FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileName) == 0) {
- NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
- if (NewFile == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }

- NewFile->Signature = FVFS_FILE_SIGNATURE;
- NewFile->Instance = Instance;
- NewFile->FvFileInfo = FvFileInfo;
- CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
- InitializeListHead (&NewFile->Link);
- InsertHeadList (&Instance->FileHead, &NewFile->Link);
+ // If the file has not been found check if the filename exists with
+ an extension // in case there was no extension present.
+ // FvFileSystem adds a 'virtual' extension '.EFI' to EFI applications
+ and drivers // present in the Firmware Volume if (Status ==
+ EFI_NOT_FOUND) {
+ FileNameLength = StrLen (FileName);
+
+ // Does the filename already contain the '.EFI' extension?
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, FileName + FileNameLength - 4, L".efi") != 0) {
+ // No, there was no extension. So add one and search again for the file
+ // NewFileNameLength = FileNameLength + 1 + 4 = (Number of non-null character) + (file extension) + (a null character)
+ NewFileNameLength = FileNameLength + 1 + 4;
+ FileNameWithExtension = AllocateCopyPool (NewFileNameLength * 2, FileName);
+ StrCatS (FileNameWithExtension, NewFileNameLength, L".EFI");
+
+ for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
+ !IsNull (&Instance->FileInfoHead, FvFileInfoLink);
+ FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
+ FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileNameWithExtension) == 0) {
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }
+ }
+ }

- *NewHandle = &NewFile->FileProtocol;
- return EFI_SUCCESS;
+ if (!EFI_ERROR (Status)) {
+ NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
+ if (NewFile == NULL) {
+ return EFI_OUT_OF_RESOURCES;
}
+
+ NewFile->Signature = FVFS_FILE_SIGNATURE;
+ NewFile->Instance = Instance;
+ NewFile->FvFileInfo = FvFileInfo;
+ CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
+ InitializeListHead (&NewFile->Link);
+ InsertHeadList (&Instance->FileHead, &NewFile->Link);
+
+ *NewHandle = &NewFile->FileProtocol;
+ return EFI_SUCCESS;
}

return EFI_NOT_FOUND;
--
2.1.1


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782

[Olivier Martin]

I would say if should be done by the callee because this behaviour is only specific to FvSimpleFileSystemDxe.
In fact it is FvSimpleFileSystemDxe who lies about the real name of the application. That's why I think it should be done by this driver :-)

-----Original Message-----
From: Tian, Feng [mailto:***@intel.com]
Sent: 09 July 2015 10:09
To: Olivier Martin
Cc: edk2-***@lists.sourceforge.net; Tian, Feng
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Martin,

Got your point now.

Do you think such behavior (extension matching) should be done in caller or callee? From ShellPkg implementation, it's done by caller.

But if you vote the callee to do this, I am also ok with your patch:). Reveiwed-by: Feng Tian <***@intel.com>

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Thursday, July 09, 2015 16:53
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Can you try without using EFI Shell?
If you check at the code of FvSimpleFileSystemOpen(), there is no way it works.
I am guessing EFI Shell try to open the file with and without '.EFI' extension.

And actually, after searching quickly in ShellPkg, you can find:

/**
Find a file by searching the CWD and then the path with a variable set of file
extensions. If the file is not found it will append each extension in the list
in the order provided and return the first one that is successful.

If FileName is NULL, then ASSERT.
If FileExtension is NULL, then behavior is identical to ShellFindFilePath.

If the return value is not NULL then the memory must be caller freed.

@param[in] FileName Filename string.
@param[in] FileExtension Semi-colon delimeted list of possible extensions.

@retval NULL The file was not found.
@retval !NULL The path to the file.
**/
CHAR16 *
EFIAPI
ShellFindFilePathEx (
IN CONST CHAR16 *FileName,
IN CONST CHAR16 *FileExtension
)

-----Original Message-----
From: Tian, Feng [mailto:***@intel.com]
Sent: 09 July 2015 09:39
To: Olivier Martin
Cc: edk2-***@lists.sourceforge.net; Tian, Feng
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Hi, Martin

I tested old code behavior on NT32 platform by adding MdeModulePkg/Application/VariableInfo/VariableInfo.inf & MdeModulePkg\Universal\FvSimpleFileSystemDxe\FvSimpleFileSystemDxe.inf to DSC&FDF files.

Under shell, the application VariableInfo could run by enter "VariableInfo" or "VariableInfo.efi" cmd.

Do I misunderstand something?

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Thursday, July 09, 2015 00:32
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

You are saying "If user enter "Shell" or "Shell.efi", it should run for both cases."
But it is actually the case I am trying to solve with this patch :-)

FvSimpleFileSystemOpen() was only checking if there was strict equality between the given name and the name exposed by FvSimpleFileSystemDxe (ie: the name with .EFI).
It means if you try to open the file "Shell" (the name you gave in the UI section) it would not work.

-----Original Message-----
From: Tian, Feng [mailto:***@intel.com]
Sent: 08 July 2015 06:38
To: Olivier Martin
Cc: edk2-***@lists.sourceforge.net; Tian, Feng
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Hi, Martin

I am a little confused by your description on the issue.

EDKII build system puts $BASE_NAME string to UI section and generate $BASE_NAME.efi file name at build directory. The FvSimpleFileSystem driver adds .efi postfix to string gotten from each UI section for all executable file types. So my understanding is these two strings are identical and should not bring confusion.

And why it's not possible to open an EFI application without the .efi extension? If user enter "Shell" or "Shell.efi", it should run for both cases.

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Tuesday, July 07, 2015 00:17
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net; Olivier Martin; Olivier Martin
Subject: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

FvSimpleFileSystem adds '.efi' to the EFI application and drivers filenames even through this extension is not present in the real filename of the EFI module.

In the current behaviour, it would not be possible to open an EFI application using FvSimpleFileSystem if the extension has been omitted in the given filename.
It can be create some confusion if someone wants to try to open a file with the real application name (eg: 'Shell').

This patch adds support to try again to look for the file with the extension if it had failed to find it without the extension.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Olivier Martin <***@arm.com>
---
.../FvSimpleFileSystemDxe/FvSimpleFileSystem.c | 62 +++++++++++++++++-----
1 file changed, 50 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
index b0e7dc3..c6137ac 100644
--- a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
+++ b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
@@ -483,6 +483,10 @@ FvSimpleFileSystemOpen (
FV_FILESYSTEM_FILE *NewFile;
FV_FILESYSTEM_FILE_INFO *FvFileInfo;
LIST_ENTRY *FvFileInfoLink;
+ EFI_STATUS Status;
+ UINTN FileNameLength;
+ UINTN NewFileNameLength;
+ CHAR16 *FileNameWithExtension;

//
// Check for a valid mode
@@ -531,26 +535,60 @@ FvSimpleFileSystemOpen (
//
// Do a linear search for a file in the FV with a matching filename
//
+ Status = EFI_NOT_FOUND;
+ FvFileInfo = NULL;
for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
!IsNull (&Instance->FileInfoHead, FvFileInfoLink);
FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileName) == 0) {
- NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
- if (NewFile == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }

- NewFile->Signature = FVFS_FILE_SIGNATURE;
- NewFile->Instance = Instance;
- NewFile->FvFileInfo = FvFileInfo;
- CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
- InitializeListHead (&NewFile->Link);
- InsertHeadList (&Instance->FileHead, &NewFile->Link);
+ // If the file has not been found check if the filename exists with
+ an extension // in case there was no extension present.
+ // FvFileSystem adds a 'virtual' extension '.EFI' to EFI applications
+ and drivers // present in the Firmware Volume if (Status ==
+ EFI_NOT_FOUND) {
+ FileNameLength = StrLen (FileName);
+
+ // Does the filename already contain the '.EFI' extension?
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, FileName + FileNameLength - 4, L".efi") != 0) {
+ // No, there was no extension. So add one and search again for the file
+ // NewFileNameLength = FileNameLength + 1 + 4 = (Number of non-null character) + (file extension) + (a null character)
+ NewFileNameLength = FileNameLength + 1 + 4;
+ FileNameWithExtension = AllocateCopyPool (NewFileNameLength * 2, FileName);
+ StrCatS (FileNameWithExtension, NewFileNameLength, L".EFI");
+
+ for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
+ !IsNull (&Instance->FileInfoHead, FvFileInfoLink);
+ FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
+ FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileNameWithExtension) == 0) {
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }
+ }
+ }

- *NewHandle = &NewFile->FileProtocol;
- return EFI_SUCCESS;
+ if (!EFI_ERROR (Status)) {
+ NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
+ if (NewFile == NULL) {
+ return EFI_OUT_OF_RESOURCES;
}
+
+ NewFile->Signature = FVFS_FILE_SIGNATURE;
+ NewFile->Instance = Instance;
+ NewFile->FvFileInfo = FvFileInfo;
+ CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
+ InitializeListHead (&NewFile->Link);
+ InsertHeadList (&Instance->FileHead, &NewFile->Link);
+
+ *NewHandle = &NewFile->FileProtocol;
+ return EFI_SUCCESS;
}

return EFI_NOT_FOUND;
--
2.1.1


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782

[Tian, Feng]

ok on that.

Could you please check in the fix?

Reviewed-by: Feng Tian <***@intel.com>

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Thursday, July 09, 2015 18:31
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

I would say if should be done by the callee because this behaviour is only specific to FvSimpleFileSystemDxe.
In fact it is FvSimpleFileSystemDxe who lies about the real name of the application. That's why I think it should be done by this driver :-)

-----Original Message-----
From: Tian, Feng [mailto:***@intel.com]
Sent: 09 July 2015 10:09
To: Olivier Martin
Cc: edk2-***@lists.sourceforge.net; Tian, Feng
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Martin,

Got your point now.

Do you think such behavior (extension matching) should be done in caller or callee? From ShellPkg implementation, it's done by caller.

But if you vote the callee to do this, I am also ok with your patch:). Reveiwed-by: Feng Tian <***@intel.com>

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Thursday, July 09, 2015 16:53
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Can you try without using EFI Shell?
If you check at the code of FvSimpleFileSystemOpen(), there is no way it works.
I am guessing EFI Shell try to open the file with and without '.EFI' extension.

And actually, after searching quickly in ShellPkg, you can find:

/**
Find a file by searching the CWD and then the path with a variable set of file
extensions. If the file is not found it will append each extension in the list
in the order provided and return the first one that is successful.

If FileName is NULL, then ASSERT.
If FileExtension is NULL, then behavior is identical to ShellFindFilePath.

If the return value is not NULL then the memory must be caller freed.

@param[in] FileName Filename string.
@param[in] FileExtension Semi-colon delimeted list of possible extensions.

@retval NULL The file was not found.
@retval !NULL The path to the file.
**/
CHAR16 *
EFIAPI
ShellFindFilePathEx (
IN CONST CHAR16 *FileName,
IN CONST CHAR16 *FileExtension
)

-----Original Message-----
From: Tian, Feng [mailto:***@intel.com]
Sent: 09 July 2015 09:39
To: Olivier Martin
Cc: edk2-***@lists.sourceforge.net; Tian, Feng
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Hi, Martin

I tested old code behavior on NT32 platform by adding MdeModulePkg/Application/VariableInfo/VariableInfo.inf & MdeModulePkg\Universal\FvSimpleFileSystemDxe\FvSimpleFileSystemDxe.inf to DSC&FDF files.

Under shell, the application VariableInfo could run by enter "VariableInfo" or "VariableInfo.efi" cmd.

Do I misunderstand something?

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Thursday, July 09, 2015 00:32
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

You are saying "If user enter "Shell" or "Shell.efi", it should run for both cases."
But it is actually the case I am trying to solve with this patch :-)

FvSimpleFileSystemOpen() was only checking if there was strict equality between the given name and the name exposed by FvSimpleFileSystemDxe (ie: the name with .EFI).
It means if you try to open the file "Shell" (the name you gave in the UI section) it would not work.

-----Original Message-----
From: Tian, Feng [mailto:***@intel.com]
Sent: 08 July 2015 06:38
To: Olivier Martin
Cc: edk2-***@lists.sourceforge.net; Tian, Feng
Subject: RE: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

Hi, Martin

I am a little confused by your description on the issue.

EDKII build system puts $BASE_NAME string to UI section and generate $BASE_NAME.efi file name at build directory. The FvSimpleFileSystem driver adds .efi postfix to string gotten from each UI section for all executable file types. So my understanding is these two strings are identical and should not bring confusion.

And why it's not possible to open an EFI application without the .efi extension? If user enter "Shell" or "Shell.efi", it should run for both cases.

Thanks
Feng

-----Original Message-----
From: Olivier Martin [mailto:***@arm.com]
Sent: Tuesday, July 07, 2015 00:17
To: Tian, Feng
Cc: edk2-***@lists.sourceforge.net; Olivier Martin; Olivier Martin
Subject: [PATCH] MdeModulePkg/FvSimpleFileSystemDxe: Support file opening with no '.efi'

FvSimpleFileSystem adds '.efi' to the EFI application and drivers filenames even through this extension is not present in the real filename of the EFI module.

In the current behaviour, it would not be possible to open an EFI application using FvSimpleFileSystem if the extension has been omitted in the given filename.
It can be create some confusion if someone wants to try to open a file with the real application name (eg: 'Shell').

This patch adds support to try again to look for the file with the extension if it had failed to find it without the extension.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Olivier Martin <***@arm.com>
---
.../FvSimpleFileSystemDxe/FvSimpleFileSystem.c | 62 +++++++++++++++++-----
1 file changed, 50 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
index b0e7dc3..c6137ac 100644
--- a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
+++ b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
@@ -483,6 +483,10 @@ FvSimpleFileSystemOpen (
FV_FILESYSTEM_FILE *NewFile;
FV_FILESYSTEM_FILE_INFO *FvFileInfo;
LIST_ENTRY *FvFileInfoLink;
+ EFI_STATUS Status;
+ UINTN FileNameLength;
+ UINTN NewFileNameLength;
+ CHAR16 *FileNameWithExtension;

//
// Check for a valid mode
@@ -531,26 +535,60 @@ FvSimpleFileSystemOpen (
//
// Do a linear search for a file in the FV with a matching filename
//
+ Status = EFI_NOT_FOUND;
+ FvFileInfo = NULL;
for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
!IsNull (&Instance->FileInfoHead, FvFileInfoLink);
FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileName) == 0) {
- NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
- if (NewFile == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }

- NewFile->Signature = FVFS_FILE_SIGNATURE;
- NewFile->Instance = Instance;
- NewFile->FvFileInfo = FvFileInfo;
- CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
- InitializeListHead (&NewFile->Link);
- InsertHeadList (&Instance->FileHead, &NewFile->Link);
+ // If the file has not been found check if the filename exists with
+ an extension // in case there was no extension present.
+ // FvFileSystem adds a 'virtual' extension '.EFI' to EFI applications
+ and drivers // present in the Firmware Volume if (Status ==
+ EFI_NOT_FOUND) {
+ FileNameLength = StrLen (FileName);
+
+ // Does the filename already contain the '.EFI' extension?
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, FileName + FileNameLength - 4, L".efi") != 0) {
+ // No, there was no extension. So add one and search again for the file
+ // NewFileNameLength = FileNameLength + 1 + 4 = (Number of non-null character) + (file extension) + (a null character)
+ NewFileNameLength = FileNameLength + 1 + 4;
+ FileNameWithExtension = AllocateCopyPool (NewFileNameLength * 2, FileName);
+ StrCatS (FileNameWithExtension, NewFileNameLength, L".EFI");
+
+ for (FvFileInfoLink = GetFirstNode (&Instance->FileInfoHead);
+ !IsNull (&Instance->FileInfoHead, FvFileInfoLink);
+ FvFileInfoLink = GetNextNode (&Instance->FileInfoHead, FvFileInfoLink)) {
+ FvFileInfo = FVFS_FILE_INFO_FROM_LINK (FvFileInfoLink);
+ if (mUnicodeCollation->StriColl (mUnicodeCollation, &FvFileInfo->FileInfo.FileName[0], FileNameWithExtension) == 0) {
+ Status = EFI_SUCCESS;
+ break;
+ }
+ }
+ }
+ }

- *NewHandle = &NewFile->FileProtocol;
- return EFI_SUCCESS;
+ if (!EFI_ERROR (Status)) {
+ NewFile = AllocateZeroPool (sizeof (FV_FILESYSTEM_FILE));
+ if (NewFile == NULL) {
+ return EFI_OUT_OF_RESOURCES;
}
+
+ NewFile->Signature = FVFS_FILE_SIGNATURE;
+ NewFile->Instance = Instance;
+ NewFile->FvFileInfo = FvFileInfo;
+ CopyMem (&NewFile->FileProtocol, &mFileSystemTemplate, sizeof (mFileSystemTemplate));
+ InitializeListHead (&NewFile->Link);
+ InsertHeadList (&Instance->FileHead, &NewFile->Link);
+
+ *NewHandle = &NewFile->FileProtocol;
+ return EFI_SUCCESS;
}

return EFI_NOT_FOUND;
--
2.1.1


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782