Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
On 08/02/18 06:06, Gao, Liming wrote:
> Laszlo:
> I have no other comments. IntelFrameworkPkg has another UefiLib library
> instance FrameworkUefiLib. Could you help update it also?
>
> For MdePkg, you can add Reviewed-by: Liming Gao
Thanks! I have now enough feedback for posting v2.
Cheers
Laszlo
>
> Thanks
> Liming
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Monday, July 30, 2018 10:14 PM
>> To: Ni, Ruiyu ; edk2-devel-01
>> Cc: Zhang, Chao B ; Dong, Eric
>> ; Carsey, Jaben ; Wu, Jiaxin
>> ; Yao, Jiewen ; Gao, Liming
>> ; Kinney, Michael D ;
>> Roman Bacik ; Fu, Siyuan
>> ; Zeng, Star
>> Subject: Re: [PATCH 1/6] MdePkg/UefiLib: introduce
>> EfiOpenFileByDevicePath()
>>
>> On 07/30/18 03:54, Ni, Ruiyu wrote:
>>> On 7/27/2018 8:06 PM, Laszlo Ersek wrote:
On 07/27/18 11:28, Ni, Ruiyu wrote:
> On 7/19/2018 4:50 AM, Laszlo Ersek wrote:
>
>> + //
>> + // Traverse the device path nodes relative to the filesystem.
>> + //
>> + while (!IsDevicePathEnd (*FilePath)) {
>> + //
>> + // Keep local variables that relate to the current device path
>> node tightly
>> + // scoped.
>> + //
>> + FILEPATH_DEVICE_PATH *FilePathNode;
>> + CHAR16 *AlignedPathName;
>> + CHAR16 *PathName;
>> + EFI_FILE_PROTOCOL *NextFile;
> 1. Not sure if it follows the coding style. I would prefer to move the
> definition to the beginning of the function.
OK, will do.
>>>
>>> Thanks!
>>>
>
>> +
>> + if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
>> + DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP) {
>> + Status = EFI_INVALID_PARAMETER;
>> + goto CloseLastFile;
>> + }
>> + FilePathNode = (FILEPATH_DEVICE_PATH *)*FilePath;
>> +
>> + //
>> + // FilePathNode->PathName may be unaligned, and the UEFI
>> specification
>> + // requires pointers that are passed to protocol member functions
>> to be
>> + // aligned. Create an aligned copy of the pathname if necessary.
>> + //
>> + if ((UINTN)FilePathNode->PathName % sizeof
>> *FilePathNode->PathName == 0) {
>> + AlignedPathName = NULL;
>> + PathName = FilePathNode->PathName;
>> + } else {
>> + AlignedPathName = AllocateCopyPool (
>> + (DevicePathNodeLength (FilePathNode) -
>> + SIZE_OF_FILEPATH_DEVICE_PATH),
>> + FilePathNode->PathName
>> + );
>> + if (AlignedPathName == NULL) {
>> + Status = EFI_OUT_OF_RESOURCES;
>> + goto CloseLastFile;
>> + }
>> + PathName = AlignedPathName;
>> + }
>> +
>> + //
>> + // Open the next pathname fragment with EFI_FILE_MODE_CREATE
>> masked out and
>> + // with Attributes set to 0.
>> + //
>> + Status = LastFile->Open (
>> + LastFile,
>> + ,
>> + PathName,
>> + OpenMode & ~(UINT64)EFI_FILE_MODE_CREATE,
>> + 0
>> + );
> 2. As I said in previous mail, is it really needed?
> Per spec it's not required. Per FAT driver implementation, it's also not
> required.
I can do that, but it's out of scope for this series. The behavior that
you point out is not a functionality bug (it is not observably erroneous
behavior), just sub-optimal implementation. This series is about
unifying the implementation and fixing those issues that are actual
bugs. I suggest that we report a separate BZ about this question,
discuss it separately, and then I can send a separate patch (which will
benefit all client code at once).
Does that sound acceptable?
>>>
>>> I agree.
>>>
>
>> +
>> + //
>> + // Retry with EFI_FILE_MODE_CREATE and the original Attributes if
>> the first
>> + // attempt failed, and the caller specified EFI_FILE_MODE_CREATE.
>> + //
>> + if (EFI_ERROR (Status) && (OpenMode &
>> EFI_FILE_MODE_CREATE) !=
>> 0) {
>> + Status = LastFile->Open (
>> + LastFile,
>> + ,
>> + PathName,
>> + OpenMode,
>> + Attributes
>> + );
>> + }
>> +
>> + //
>> + // Release any AlignedPathName on both error and success paths;
>> PathName is
>> + // no longer needed.
>> + //
>> + if (AlignedPathName != NULL) {
>> + FreePool (AlignedPathName);
>> + }
>> + if (EFI_ERROR (Status)) {
>> + goto
Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
Laszlo:
I have no other comments. IntelFrameworkPkg has another UefiLib library
instance FrameworkUefiLib. Could you help update it also?
For MdePkg, you can add Reviewed-by: Liming Gao
Thanks
Liming
>-Original Message-
>From: Laszlo Ersek [mailto:ler...@redhat.com]
>Sent: Monday, July 30, 2018 10:14 PM
>To: Ni, Ruiyu ; edk2-devel-01
>Cc: Zhang, Chao B ; Dong, Eric
>; Carsey, Jaben ; Wu, Jiaxin
>; Yao, Jiewen ; Gao, Liming
>; Kinney, Michael D ;
>Roman Bacik ; Fu, Siyuan
>; Zeng, Star
>Subject: Re: [PATCH 1/6] MdePkg/UefiLib: introduce
>EfiOpenFileByDevicePath()
>
>On 07/30/18 03:54, Ni, Ruiyu wrote:
>> On 7/27/2018 8:06 PM, Laszlo Ersek wrote:
>>> On 07/27/18 11:28, Ni, Ruiyu wrote:
On 7/19/2018 4:50 AM, Laszlo Ersek wrote:
> + //
> + // Traverse the device path nodes relative to the filesystem.
> + //
> + while (!IsDevicePathEnd (*FilePath)) {
> + //
> + // Keep local variables that relate to the current device path
> node tightly
> + // scoped.
> + //
> + FILEPATH_DEVICE_PATH *FilePathNode;
> + CHAR16 *AlignedPathName;
> + CHAR16 *PathName;
> + EFI_FILE_PROTOCOL *NextFile;
1. Not sure if it follows the coding style. I would prefer to move the
definition to the beginning of the function.
>>>
>>> OK, will do.
>>
>> Thanks!
>>
>>>
> +
> + if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
> + DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP) {
> + Status = EFI_INVALID_PARAMETER;
> + goto CloseLastFile;
> + }
> + FilePathNode = (FILEPATH_DEVICE_PATH *)*FilePath;
> +
> + //
> + // FilePathNode->PathName may be unaligned, and the UEFI
> specification
> + // requires pointers that are passed to protocol member functions
> to be
> + // aligned. Create an aligned copy of the pathname if necessary.
> + //
> + if ((UINTN)FilePathNode->PathName % sizeof
> *FilePathNode->PathName == 0) {
> + AlignedPathName = NULL;
> + PathName = FilePathNode->PathName;
> + } else {
> + AlignedPathName = AllocateCopyPool (
> + (DevicePathNodeLength (FilePathNode) -
> + SIZE_OF_FILEPATH_DEVICE_PATH),
> + FilePathNode->PathName
> + );
> + if (AlignedPathName == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + goto CloseLastFile;
> + }
> + PathName = AlignedPathName;
> + }
> +
> + //
> + // Open the next pathname fragment with EFI_FILE_MODE_CREATE
> masked out and
> + // with Attributes set to 0.
> + //
> + Status = LastFile->Open (
> + LastFile,
> + ,
> + PathName,
> + OpenMode & ~(UINT64)EFI_FILE_MODE_CREATE,
> + 0
> + );
2. As I said in previous mail, is it really needed?
Per spec it's not required. Per FAT driver implementation, it's also not
required.
>>>
>>> I can do that, but it's out of scope for this series. The behavior that
>>> you point out is not a functionality bug (it is not observably erroneous
>>> behavior), just sub-optimal implementation. This series is about
>>> unifying the implementation and fixing those issues that are actual
>>> bugs. I suggest that we report a separate BZ about this question,
>>> discuss it separately, and then I can send a separate patch (which will
>>> benefit all client code at once).
>>>
>>> Does that sound acceptable?
>>
>> I agree.
>>
>>>
> +
> + //
> + // Retry with EFI_FILE_MODE_CREATE and the original Attributes if
> the first
> + // attempt failed, and the caller specified EFI_FILE_MODE_CREATE.
> + //
> + if (EFI_ERROR (Status) && (OpenMode &
>EFI_FILE_MODE_CREATE) !=
> 0) {
> + Status = LastFile->Open (
> + LastFile,
> + ,
> + PathName,
> + OpenMode,
> + Attributes
> + );
> + }
> +
> + //
> + // Release any AlignedPathName on both error and success paths;
> PathName is
> + // no longer needed.
> + //
> + if (AlignedPathName != NULL) {
> + FreePool (AlignedPathName);
> + }
> + if (EFI_ERROR (Status)) {
> + goto CloseLastFile;
> + }
> +
> + //
> + // Advance to the next device path node.
> + //
> + LastFile->Close (LastFile);
> + LastFile = NextFile;
> + *FilePath = NextDevicePathNode (FilePathNode);
> + }
> +
> +
Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
On 07/30/18 03:54, Ni, Ruiyu wrote:
> On 7/27/2018 8:06 PM, Laszlo Ersek wrote:
>> On 07/27/18 11:28, Ni, Ruiyu wrote:
>>> On 7/19/2018 4:50 AM, Laszlo Ersek wrote:
>>>
+ //
+ // Traverse the device path nodes relative to the filesystem.
+ //
+ while (!IsDevicePathEnd (*FilePath)) {
+ //
+ // Keep local variables that relate to the current device path
node tightly
+ // scoped.
+ //
+ FILEPATH_DEVICE_PATH *FilePathNode;
+ CHAR16 *AlignedPathName;
+ CHAR16 *PathName;
+ EFI_FILE_PROTOCOL *NextFile;
>>> 1. Not sure if it follows the coding style. I would prefer to move the
>>> definition to the beginning of the function.
>>
>> OK, will do.
>
> Thanks!
>
>>
>>>
+
+ if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
+ DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP) {
+ Status = EFI_INVALID_PARAMETER;
+ goto CloseLastFile;
+ }
+ FilePathNode = (FILEPATH_DEVICE_PATH *)*FilePath;
+
+ //
+ // FilePathNode->PathName may be unaligned, and the UEFI
specification
+ // requires pointers that are passed to protocol member functions
to be
+ // aligned. Create an aligned copy of the pathname if necessary.
+ //
+ if ((UINTN)FilePathNode->PathName % sizeof
*FilePathNode->PathName == 0) {
+ AlignedPathName = NULL;
+ PathName = FilePathNode->PathName;
+ } else {
+ AlignedPathName = AllocateCopyPool (
+ (DevicePathNodeLength (FilePathNode) -
+ SIZE_OF_FILEPATH_DEVICE_PATH),
+ FilePathNode->PathName
+ );
+ if (AlignedPathName == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto CloseLastFile;
+ }
+ PathName = AlignedPathName;
+ }
+
+ //
+ // Open the next pathname fragment with EFI_FILE_MODE_CREATE
masked out and
+ // with Attributes set to 0.
+ //
+ Status = LastFile->Open (
+ LastFile,
+ ,
+ PathName,
+ OpenMode & ~(UINT64)EFI_FILE_MODE_CREATE,
+ 0
+ );
>>> 2. As I said in previous mail, is it really needed?
>>> Per spec it's not required. Per FAT driver implementation, it's also not
>>> required.
>>
>> I can do that, but it's out of scope for this series. The behavior that
>> you point out is not a functionality bug (it is not observably erroneous
>> behavior), just sub-optimal implementation. This series is about
>> unifying the implementation and fixing those issues that are actual
>> bugs. I suggest that we report a separate BZ about this question,
>> discuss it separately, and then I can send a separate patch (which will
>> benefit all client code at once).
>>
>> Does that sound acceptable?
>
> I agree.
>
>>
>>>
+
+ //
+ // Retry with EFI_FILE_MODE_CREATE and the original Attributes if
the first
+ // attempt failed, and the caller specified EFI_FILE_MODE_CREATE.
+ //
+ if (EFI_ERROR (Status) && (OpenMode & EFI_FILE_MODE_CREATE) !=
0) {
+ Status = LastFile->Open (
+ LastFile,
+ ,
+ PathName,
+ OpenMode,
+ Attributes
+ );
+ }
+
+ //
+ // Release any AlignedPathName on both error and success paths;
PathName is
+ // no longer needed.
+ //
+ if (AlignedPathName != NULL) {
+ FreePool (AlignedPathName);
+ }
+ if (EFI_ERROR (Status)) {
+ goto CloseLastFile;
+ }
+
+ //
+ // Advance to the next device path node.
+ //
+ LastFile->Close (LastFile);
+ LastFile = NextFile;
+ *FilePath = NextDevicePathNode (FilePathNode);
+ }
+
+ *File = LastFile;
+ return EFI_SUCCESS;
+
+CloseLastFile:
+ LastFile->Close (LastFile);
+
+ ASSERT (EFI_ERROR (Status));
>>> 3. ASSERT_EFI_ERROR (Status);
>>
>> No, that's not correct; I *really* meant
>>
>> ASSERT (EFI_ERROR (Status))
>>
>> Please find the explanation here:
>>
>> https://lists.01.org/pipermail/edk2-devel/2018-July/027288.html
>>
>> However, given that both Jaben and you were confused by this, I agree
>> that I should add a comment before the assert:
>>
>> //
>> // We are on the error path; we must have set an error Status for
>> // returning to the caller.
>> //
>
> I just found there is no "!" before "EFI_ERROR".
> It's really
Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
On 7/27/2018 8:06 PM, Laszlo Ersek wrote:
On 07/27/18 11:28, Ni, Ruiyu wrote:
On 7/19/2018 4:50 AM, Laszlo Ersek wrote:
+ //
+ // Traverse the device path nodes relative to the filesystem.
+ //
+ while (!IsDevicePathEnd (*FilePath)) {
+ //
+ // Keep local variables that relate to the current device path
node tightly
+ // scoped.
+ //
+ FILEPATH_DEVICE_PATH *FilePathNode;
+ CHAR16 *AlignedPathName;
+ CHAR16 *PathName;
+ EFI_FILE_PROTOCOL *NextFile;
1. Not sure if it follows the coding style. I would prefer to move the
definition to the beginning of the function.
OK, will do.
Thanks!
+
+ if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
+ DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP) {
+ Status = EFI_INVALID_PARAMETER;
+ goto CloseLastFile;
+ }
+ FilePathNode = (FILEPATH_DEVICE_PATH *)*FilePath;
+
+ //
+ // FilePathNode->PathName may be unaligned, and the UEFI
specification
+ // requires pointers that are passed to protocol member functions
to be
+ // aligned. Create an aligned copy of the pathname if necessary.
+ //
+ if ((UINTN)FilePathNode->PathName % sizeof
*FilePathNode->PathName == 0) {
+ AlignedPathName = NULL;
+ PathName = FilePathNode->PathName;
+ } else {
+ AlignedPathName = AllocateCopyPool (
+ (DevicePathNodeLength (FilePathNode) -
+ SIZE_OF_FILEPATH_DEVICE_PATH),
+ FilePathNode->PathName
+ );
+ if (AlignedPathName == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto CloseLastFile;
+ }
+ PathName = AlignedPathName;
+ }
+
+ //
+ // Open the next pathname fragment with EFI_FILE_MODE_CREATE
masked out and
+ // with Attributes set to 0.
+ //
+ Status = LastFile->Open (
+ LastFile,
+ ,
+ PathName,
+ OpenMode & ~(UINT64)EFI_FILE_MODE_CREATE,
+ 0
+ );
2. As I said in previous mail, is it really needed?
Per spec it's not required. Per FAT driver implementation, it's also not
required.
I can do that, but it's out of scope for this series. The behavior that
you point out is not a functionality bug (it is not observably erroneous
behavior), just sub-optimal implementation. This series is about
unifying the implementation and fixing those issues that are actual
bugs. I suggest that we report a separate BZ about this question,
discuss it separately, and then I can send a separate patch (which will
benefit all client code at once).
Does that sound acceptable?
I agree.
+
+ //
+ // Retry with EFI_FILE_MODE_CREATE and the original Attributes if
the first
+ // attempt failed, and the caller specified EFI_FILE_MODE_CREATE.
+ //
+ if (EFI_ERROR (Status) && (OpenMode & EFI_FILE_MODE_CREATE) != 0) {
+ Status = LastFile->Open (
+ LastFile,
+ ,
+ PathName,
+ OpenMode,
+ Attributes
+ );
+ }
+
+ //
+ // Release any AlignedPathName on both error and success paths;
PathName is
+ // no longer needed.
+ //
+ if (AlignedPathName != NULL) {
+ FreePool (AlignedPathName);
+ }
+ if (EFI_ERROR (Status)) {
+ goto CloseLastFile;
+ }
+
+ //
+ // Advance to the next device path node.
+ //
+ LastFile->Close (LastFile);
+ LastFile = NextFile;
+ *FilePath = NextDevicePathNode (FilePathNode);
+ }
+
+ *File = LastFile;
+ return EFI_SUCCESS;
+
+CloseLastFile:
+ LastFile->Close (LastFile);
+
+ ASSERT (EFI_ERROR (Status));
3. ASSERT_EFI_ERROR (Status);
No, that's not correct; I *really* meant
ASSERT (EFI_ERROR (Status))
Please find the explanation here:
https://lists.01.org/pipermail/edk2-devel/2018-July/027288.html
However, given that both Jaben and you were confused by this, I agree
that I should add a comment before the assert:
//
// We are on the error path; we must have set an error Status for
// returning to the caller.
//
I just found there is no "!" before "EFI_ERROR".
It's really confusing. I agree a comment before that is better.
Thanks!
With the comment added, Reviewed-by: Ruiyu Ni
Thanks!
Laszlo
+ return Status;
+}
--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
On 07/27/18 11:28, Ni, Ruiyu wrote:
> On 7/19/2018 4:50 AM, Laszlo Ersek wrote:
>
>> + //
>> + // Traverse the device path nodes relative to the filesystem.
>> + //
>> + while (!IsDevicePathEnd (*FilePath)) {
>> + //
>> + // Keep local variables that relate to the current device path
>> node tightly
>> + // scoped.
>> + //
>> + FILEPATH_DEVICE_PATH *FilePathNode;
>> + CHAR16 *AlignedPathName;
>> + CHAR16 *PathName;
>> + EFI_FILE_PROTOCOL *NextFile;
> 1. Not sure if it follows the coding style. I would prefer to move the
> definition to the beginning of the function.
OK, will do.
>
>> +
>> + if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
>> + DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP) {
>> + Status = EFI_INVALID_PARAMETER;
>> + goto CloseLastFile;
>> + }
>> + FilePathNode = (FILEPATH_DEVICE_PATH *)*FilePath;
>> +
>> + //
>> + // FilePathNode->PathName may be unaligned, and the UEFI
>> specification
>> + // requires pointers that are passed to protocol member functions
>> to be
>> + // aligned. Create an aligned copy of the pathname if necessary.
>> + //
>> + if ((UINTN)FilePathNode->PathName % sizeof
>> *FilePathNode->PathName == 0) {
>> + AlignedPathName = NULL;
>> + PathName = FilePathNode->PathName;
>> + } else {
>> + AlignedPathName = AllocateCopyPool (
>> + (DevicePathNodeLength (FilePathNode) -
>> + SIZE_OF_FILEPATH_DEVICE_PATH),
>> + FilePathNode->PathName
>> + );
>> + if (AlignedPathName == NULL) {
>> + Status = EFI_OUT_OF_RESOURCES;
>> + goto CloseLastFile;
>> + }
>> + PathName = AlignedPathName;
>> + }
>> +
>> + //
>> + // Open the next pathname fragment with EFI_FILE_MODE_CREATE
>> masked out and
>> + // with Attributes set to 0.
>> + //
>> + Status = LastFile->Open (
>> + LastFile,
>> + ,
>> + PathName,
>> + OpenMode & ~(UINT64)EFI_FILE_MODE_CREATE,
>> + 0
>> + );
> 2. As I said in previous mail, is it really needed?
> Per spec it's not required. Per FAT driver implementation, it's also not
> required.
I can do that, but it's out of scope for this series. The behavior that
you point out is not a functionality bug (it is not observably erroneous
behavior), just sub-optimal implementation. This series is about
unifying the implementation and fixing those issues that are actual
bugs. I suggest that we report a separate BZ about this question,
discuss it separately, and then I can send a separate patch (which will
benefit all client code at once).
Does that sound acceptable?
>
>> +
>> + //
>> + // Retry with EFI_FILE_MODE_CREATE and the original Attributes if
>> the first
>> + // attempt failed, and the caller specified EFI_FILE_MODE_CREATE.
>> + //
>> + if (EFI_ERROR (Status) && (OpenMode & EFI_FILE_MODE_CREATE) != 0) {
>> + Status = LastFile->Open (
>> + LastFile,
>> + ,
>> + PathName,
>> + OpenMode,
>> + Attributes
>> + );
>> + }
>> +
>> + //
>> + // Release any AlignedPathName on both error and success paths;
>> PathName is
>> + // no longer needed.
>> + //
>> + if (AlignedPathName != NULL) {
>> + FreePool (AlignedPathName);
>> + }
>> + if (EFI_ERROR (Status)) {
>> + goto CloseLastFile;
>> + }
>> +
>> + //
>> + // Advance to the next device path node.
>> + //
>> + LastFile->Close (LastFile);
>> + LastFile = NextFile;
>> + *FilePath = NextDevicePathNode (FilePathNode);
>> + }
>> +
>> + *File = LastFile;
>> + return EFI_SUCCESS;
>> +
>> +CloseLastFile:
>> + LastFile->Close (LastFile);
>> +
>> + ASSERT (EFI_ERROR (Status));
> 3. ASSERT_EFI_ERROR (Status);
No, that's not correct; I *really* meant
ASSERT (EFI_ERROR (Status))
Please find the explanation here:
https://lists.01.org/pipermail/edk2-devel/2018-July/027288.html
However, given that both Jaben and you were confused by this, I agree
that I should add a comment before the assert:
//
// We are on the error path; we must have set an error Status for
// returning to the caller.
//
Thanks!
Laszlo
>
>> + return Status;
>> +}
>>
>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
On 7/19/2018 4:50 AM, Laszlo Ersek wrote:
+ //
+ // Traverse the device path nodes relative to the filesystem.
+ //
+ while (!IsDevicePathEnd (*FilePath)) {
+//
+// Keep local variables that relate to the current device path node tightly
+// scoped.
+//
+FILEPATH_DEVICE_PATH *FilePathNode;
+CHAR16 *AlignedPathName;
+CHAR16 *PathName;
+EFI_FILE_PROTOCOL*NextFile;
1. Not sure if it follows the coding style. I would prefer to move the
definition to the beginning of the function.
+
+if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
+DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP) {
+ Status = EFI_INVALID_PARAMETER;
+ goto CloseLastFile;
+}
+FilePathNode = (FILEPATH_DEVICE_PATH *)*FilePath;
+
+//
+// FilePathNode->PathName may be unaligned, and the UEFI specification
+// requires pointers that are passed to protocol member functions to be
+// aligned. Create an aligned copy of the pathname if necessary.
+//
+if ((UINTN)FilePathNode->PathName % sizeof *FilePathNode->PathName == 0) {
+ AlignedPathName = NULL;
+ PathName = FilePathNode->PathName;
+} else {
+ AlignedPathName = AllocateCopyPool (
+ (DevicePathNodeLength (FilePathNode) -
+ SIZE_OF_FILEPATH_DEVICE_PATH),
+ FilePathNode->PathName
+ );
+ if (AlignedPathName == NULL) {
+Status = EFI_OUT_OF_RESOURCES;
+goto CloseLastFile;
+ }
+ PathName = AlignedPathName;
+}
+
+//
+// Open the next pathname fragment with EFI_FILE_MODE_CREATE masked out and
+// with Attributes set to 0.
+//
+Status = LastFile->Open (
+ LastFile,
+ ,
+ PathName,
+ OpenMode & ~(UINT64)EFI_FILE_MODE_CREATE,
+ 0
+ );
2. As I said in previous mail, is it really needed?
Per spec it's not required. Per FAT driver implementation, it's also not
required.
+
+//
+// Retry with EFI_FILE_MODE_CREATE and the original Attributes if the first
+// attempt failed, and the caller specified EFI_FILE_MODE_CREATE.
+//
+if (EFI_ERROR (Status) && (OpenMode & EFI_FILE_MODE_CREATE) != 0) {
+ Status = LastFile->Open (
+ LastFile,
+ ,
+ PathName,
+ OpenMode,
+ Attributes
+ );
+}
+
+//
+// Release any AlignedPathName on both error and success paths; PathName is
+// no longer needed.
+//
+if (AlignedPathName != NULL) {
+ FreePool (AlignedPathName);
+}
+if (EFI_ERROR (Status)) {
+ goto CloseLastFile;
+}
+
+//
+// Advance to the next device path node.
+//
+LastFile->Close (LastFile);
+LastFile = NextFile;
+*FilePath = NextDevicePathNode (FilePathNode);
+ }
+
+ *File = LastFile;
+ return EFI_SUCCESS;
+
+CloseLastFile:
+ LastFile->Close (LastFile);
+
+ ASSERT (EFI_ERROR (Status));
3. ASSERT_EFI_ERROR (Status);
+ return Status;
+}
--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
On 7/19/2018 4:50 AM, Laszlo Ersek wrote:
+//
+// Open the next pathname fragment with EFI_FILE_MODE_CREATE masked out and
+// with Attributes set to 0.
+//
+Status = LastFile->Open (
+ LastFile,
+ ,
+ PathName,
+ OpenMode & ~(UINT64)EFI_FILE_MODE_CREATE,
+ 0
+ );
+
+//
+// Retry with EFI_FILE_MODE_CREATE and the original Attributes if the first
+// attempt failed, and the caller specified EFI_FILE_MODE_CREATE.
+//
+if (EFI_ERROR (Status) && (OpenMode & EFI_FILE_MODE_CREATE) != 0) {
+ Status = LastFile->Open (
+ LastFile,
+ ,
+ PathName,
+ OpenMode,
+ Attributes
+ );
+}
Laszlo,
Why to open the file firstly with CREATE flag off?
Per spec, when CREATE is set but file exists, the file is opened.
--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
On 07/18/18 22:50, Laszlo Ersek wrote: > The EfiOpenFileByDevicePath() function centralizes functionality from > > - MdeModulePkg/Universal/Disk/RamDiskDxe > - NetworkPkg/TlsAuthConfigDxe > - SecurityPkg/VariableAuthenticated/SecureBootConfigDxe > - ShellPkg/Library/UefiShellLib > > unifying the implementation and fixing various bugs. Mike, Liming: do you have comments on this patch? Thanks! Laszlo > > Cc: Chao Zhang > Cc: Eric Dong > Cc: Jaben Carsey > Cc: Jiaxin Wu > Cc: Jiewen Yao > Cc: Liming Gao > Cc: Michael D Kinney > Cc: Roman Bacik > Cc: Ruiyu Ni > Cc: Siyuan Fu > Cc: Star Zeng > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek > --- > MdePkg/Library/UefiLib/UefiLib.inf | 1 + > MdePkg/Include/Library/UefiLib.h | 86 > MdePkg/Library/UefiLib/UefiLib.c | 226 > 3 files changed, 313 insertions(+) > > diff --git a/MdePkg/Library/UefiLib/UefiLib.inf > b/MdePkg/Library/UefiLib/UefiLib.inf > index f69f0a43b576..a6c739ef3d6d 100644 > --- a/MdePkg/Library/UefiLib/UefiLib.inf > +++ b/MdePkg/Library/UefiLib/UefiLib.inf > @@ -68,6 +68,7 @@ [Protocols] >gEfiSimpleTextOutProtocolGuid ## SOMETIMES_CONSUMES >gEfiGraphicsOutputProtocolGuid ## SOMETIMES_CONSUMES >gEfiHiiFontProtocolGuid ## SOMETIMES_CONSUMES > + gEfiSimpleFileSystemProtocolGuid## SOMETIMES_CONSUMES >gEfiUgaDrawProtocolGuid | gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport > ## SOMETIMES_CONSUMES # Consumes if > gEfiGraphicsOutputProtocolGuid uninstalled >gEfiComponentNameProtocolGuid | NOT > gEfiMdePkgTokenSpaceGuid.PcdComponentNameDisable ## SOMETIMES_PRODUCES # > User chooses to produce it >gEfiComponentName2ProtocolGuid | NOT > gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable ## SOMETIMES_PRODUCES # > User chooses to produce it > diff --git a/MdePkg/Include/Library/UefiLib.h > b/MdePkg/Include/Library/UefiLib.h > index 7c6fde620c74..2468bf2aee80 100644 > --- a/MdePkg/Include/Library/UefiLib.h > +++ b/MdePkg/Include/Library/UefiLib.h > @@ -33,6 +33,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > #include > #include > #include > +#include > +#include > > #include > > @@ -1520,4 +1522,88 @@ EfiLocateProtocolBuffer ( >OUT UINTN *NoProtocols, >OUT VOID ***Buffer >); > + > +/** > + Open or create a file or directory, possibly creating the chain of > + directories leading up to the directory. > + > + EfiOpenFileByDevicePath() first locates EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on > + FilePath, and opens the root directory of that filesystem with > + EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.OpenVolume(). > + > + On the remaining device path, the longest initial sequence of > + FILEPATH_DEVICE_PATH nodes is node-wise traversed with > + EFI_FILE_PROTOCOL.Open(). For the pathname fragment specified by each > + traversed FILEPATH_DEVICE_PATH node, EfiOpenFileByDevicePath() first masks > + EFI_FILE_MODE_CREATE out of OpenMode, and passes 0 for Attributes. If > + EFI_FILE_PROTOCOL.Open() fails, and OpenMode includes EFI_FILE_MODE_CREATE, > + then the operation is retried with the caller's OpenMode and Attributes > + unmodified. > + > + (As a consequence, if OpenMode includes EFI_FILE_MODE_CREATE, and > Attributes > + includes EFI_FILE_DIRECTORY, and each FILEPATH_DEVICE_PATH specifies a > single > + pathname component, then EfiOpenFileByDevicePath() ensures that the > specified > + series of subdirectories exist on return.) > + > + The EFI_FILE_PROTOCOL identified by the last FILEPATH_DEVICE_PATH node is > + output to the caller; intermediate EFI_FILE_PROTOCOL instances are closed. > If > + there are no FILEPATH_DEVICE_PATH nodes past the node that identifies the > + filesystem, then the EFI_FILE_PROTOCOL of the root directory of the > + filesystem is output to the caller. If a device path node that is different > + from FILEPATH_DEVICE_PATH is encountered relative to the filesystem, the > + traversal is stopped with an error, and a NULL EFI_FILE_PROTOCOL is output. > + > + @param[in,out] FilePath On input, the device path to the file or directory > + to open or create. On output, FilePath points one > + past the last node in the original device path > that > + has been successfully processed. FilePath is set > on > + output even if EfiOpenFileByDevicePath() returns > an > + error. > + > + @param[out] File On error, File is set to NULL. On success, File is > + set to the EFI_FILE_PROTOCOL of the root directory > + of the filesystem, if there are no > + FILEPATH_DEVICE_PATH nodes
Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
Cool. Thanks! > -Original Message- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Thursday, July 19, 2018 6:47 PM > To: Yao, Jiewen > Cc: edk2-devel-01 ; Zhang, Chao B > ; Dong, Eric ; Carsey, Jaben > ; Wu, Jiaxin ; Gao, Liming > ; Kinney, Michael D ; > Roman Bacik ; Ni, Ruiyu ; > Fu, Siyuan ; Zeng, Star > Subject: Re: [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath() > > On 07/19/18 01:10, Yao, Jiewen wrote: > > Thanks Laszlo. > > > > Would you please add one line comment on the FilePath, to describe if the > FilePath is internal input or external input? As such the API consumer can > know if caller’s responsibility to verify it or callee’s responsibility. > > Good point. The caller is responsible for ensuring the well-formedness > of the device path pointed-to by (*FilePath). The function uses > DevicePathLib APIs such as NextDevicePathNode(), and if e.g. some device > path node headers are invalid (containing bogus lengths, for example), > then the function won't work as expected. > > I'll add such a comment. > > > For example, if the caller gets path from a read write variable, and input > > it > directly, the this API need validate before use. > > If the caller already does the verification, then this API can use it > > directly. > > Right. > > (A separate question would be if edk2 offers facilities for verifying > the well-formedness of any given device path -- it looks like a complex > task, to implement everywhere.) > > In this series I'd just like to extract the duplicated code to a common > location, and fix its bugs. I didn't try to change the interface > contract (just to document it more precisely). If we'd like to "armor" > this function against maliciously formatted device paths, IMO that > should be done separately. > > So I agree that the comment you suggest should be added. > > > Sanity check is just for the format, not the content. > > The question I have is: Where should the sanity check be? > > At the moment: in the caller. > > Thanks! > Laszlo > > > > > thank you! > > Yao, Jiewen > > > > > >> 在 2018年7月19日，上午4:50，Laszlo Ersek 写 > 道： > >> > >> The EfiOpenFileByDevicePath() function centralizes functionality from > >> > >> - MdeModulePkg/Universal/Disk/RamDiskDxe > >> - NetworkPkg/TlsAuthConfigDxe > >> - SecurityPkg/VariableAuthenticated/SecureBootConfigDxe > >> - ShellPkg/Library/UefiShellLib > >> > >> unifying the implementation and fixing various bugs. > >> > >> Cc: Chao Zhang > >> Cc: Eric Dong > >> Cc: Jaben Carsey > >> Cc: Jiaxin Wu > >> Cc: Jiewen Yao > >> Cc: Liming Gao > >> Cc: Michael D Kinney > >> Cc: Roman Bacik > >> Cc: Ruiyu Ni > >> Cc: Siyuan Fu > >> Cc: Star Zeng > >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008 > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Laszlo Ersek > >> --- > >> MdePkg/Library/UefiLib/UefiLib.inf | 1 + > >> MdePkg/Include/Library/UefiLib.h | 86 > >> MdePkg/Library/UefiLib/UefiLib.c | 226 > >> 3 files changed, 313 insertions(+) > >> > >> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf > b/MdePkg/Library/UefiLib/UefiLib.inf > >> index f69f0a43b576..a6c739ef3d6d 100644 > >> --- a/MdePkg/Library/UefiLib/UefiLib.inf > >> +++ b/MdePkg/Library/UefiLib/UefiLib.inf > >> @@ -68,6 +68,7 @@ [Protocols] > >> gEfiSimpleTextOutProtocolGuid ## > SOMETIMES_CONSUMES > >> gEfiGraphicsOutputProtocolGuid ## > SOMETIMES_CONSUMES > >> gEfiHiiFontProtocolGuid ## > SOMETIMES_CONSUMES > >> + gEfiSimpleFileSystemProtocolGuid## > SOMETIMES_CONSUMES > >> gEfiUgaDrawProtocolGuid | > gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport ## > SOMETIMES_CONSUMES # Consumes if gEfiGraphicsOutputProtocolGuid > uninstalled > >> gEfiComponentNameProtocolGuid | NOT > gEfiMdePkgTokenSpaceGuid.PcdComponentNameDisable ## > SOMETIMES_PRODUCES # User chooses to produce it > >> gEfiComponentName2ProtocolGuid | NOT > gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable ## > SOMETIMES_PRODUCES # User chooses to produce it > >> diff --git a/MdePkg/Include/Library/UefiLib.h > b/MdePkg/Include/Library/UefiLib.h > >> index 7c6fde620c74..2468bf2aee80 100644 > >> --- a/MdePkg/Include/Library/UefiLib.h > >> +++ b/MdePkg/Include/Library/UefiLib.h > >> @@ -33,6 +33,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY KIND, EITHER EXPRESS OR IMPLIED. > >> #include > >> #include > >> #include > >> +#include > >> +#include > >> > >> #include > >> > >> @@ -1520,4 +1522,88 @@ EfiLocateProtocolBuffer ( > >> OUT UINTN *NoProtocols, > >> OUT VOID ***Buffer > >> ); > >> + > >> +/** > >> + Open or create a file or directory, possibly creating the chain of > >> + directories leading up to the directory. > >> + > >> + EfiOpenFileByDevicePath() first locates > EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on > >> + FilePath, and opens the root directory of that
Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
On 07/19/18 01:10, Yao, Jiewen wrote: > Thanks Laszlo. > > Would you please add one line comment on the FilePath, to describe if the > FilePath is internal input or external input? As such the API consumer can > know if caller’s responsibility to verify it or callee’s responsibility. Good point. The caller is responsible for ensuring the well-formedness of the device path pointed-to by (*FilePath). The function uses DevicePathLib APIs such as NextDevicePathNode(), and if e.g. some device path node headers are invalid (containing bogus lengths, for example), then the function won't work as expected. I'll add such a comment. > For example, if the caller gets path from a read write variable, and input it > directly, the this API need validate before use. > If the caller already does the verification, then this API can use it > directly. Right. (A separate question would be if edk2 offers facilities for verifying the well-formedness of any given device path -- it looks like a complex task, to implement everywhere.) In this series I'd just like to extract the duplicated code to a common location, and fix its bugs. I didn't try to change the interface contract (just to document it more precisely). If we'd like to "armor" this function against maliciously formatted device paths, IMO that should be done separately. So I agree that the comment you suggest should be added. > Sanity check is just for the format, not the content. > The question I have is: Where should the sanity check be? At the moment: in the caller. Thanks! Laszlo > > thank you! > Yao, Jiewen > > >> 在 2018年7月19日，上午4:50，Laszlo Ersek 写道： >> >> The EfiOpenFileByDevicePath() function centralizes functionality from >> >> - MdeModulePkg/Universal/Disk/RamDiskDxe >> - NetworkPkg/TlsAuthConfigDxe >> - SecurityPkg/VariableAuthenticated/SecureBootConfigDxe >> - ShellPkg/Library/UefiShellLib >> >> unifying the implementation and fixing various bugs. >> >> Cc: Chao Zhang >> Cc: Eric Dong >> Cc: Jaben Carsey >> Cc: Jiaxin Wu >> Cc: Jiewen Yao >> Cc: Liming Gao >> Cc: Michael D Kinney >> Cc: Roman Bacik >> Cc: Ruiyu Ni >> Cc: Siyuan Fu >> Cc: Star Zeng >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008 >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Laszlo Ersek >> --- >> MdePkg/Library/UefiLib/UefiLib.inf | 1 + >> MdePkg/Include/Library/UefiLib.h | 86 >> MdePkg/Library/UefiLib/UefiLib.c | 226 >> 3 files changed, 313 insertions(+) >> >> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf >> b/MdePkg/Library/UefiLib/UefiLib.inf >> index f69f0a43b576..a6c739ef3d6d 100644 >> --- a/MdePkg/Library/UefiLib/UefiLib.inf >> +++ b/MdePkg/Library/UefiLib/UefiLib.inf >> @@ -68,6 +68,7 @@ [Protocols] >> gEfiSimpleTextOutProtocolGuid ## SOMETIMES_CONSUMES >> gEfiGraphicsOutputProtocolGuid ## SOMETIMES_CONSUMES >> gEfiHiiFontProtocolGuid ## SOMETIMES_CONSUMES >> + gEfiSimpleFileSystemProtocolGuid## SOMETIMES_CONSUMES >> gEfiUgaDrawProtocolGuid | gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport >> ## SOMETIMES_CONSUMES # Consumes if >> gEfiGraphicsOutputProtocolGuid uninstalled >> gEfiComponentNameProtocolGuid | NOT >> gEfiMdePkgTokenSpaceGuid.PcdComponentNameDisable ## SOMETIMES_PRODUCES # >> User chooses to produce it >> gEfiComponentName2ProtocolGuid | NOT >> gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable ## SOMETIMES_PRODUCES # >> User chooses to produce it >> diff --git a/MdePkg/Include/Library/UefiLib.h >> b/MdePkg/Include/Library/UefiLib.h >> index 7c6fde620c74..2468bf2aee80 100644 >> --- a/MdePkg/Include/Library/UefiLib.h >> +++ b/MdePkg/Include/Library/UefiLib.h >> @@ -33,6 +33,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER >> EXPRESS OR IMPLIED. >> #include >> #include >> #include >> +#include >> +#include >> >> #include >> >> @@ -1520,4 +1522,88 @@ EfiLocateProtocolBuffer ( >> OUT UINTN *NoProtocols, >> OUT VOID ***Buffer >> ); >> + >> +/** >> + Open or create a file or directory, possibly creating the chain of >> + directories leading up to the directory. >> + >> + EfiOpenFileByDevicePath() first locates EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on >> + FilePath, and opens the root directory of that filesystem with >> + EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.OpenVolume(). >> + >> + On the remaining device path, the longest initial sequence of >> + FILEPATH_DEVICE_PATH nodes is node-wise traversed with >> + EFI_FILE_PROTOCOL.Open(). For the pathname fragment specified by each >> + traversed FILEPATH_DEVICE_PATH node, EfiOpenFileByDevicePath() first masks >> + EFI_FILE_MODE_CREATE out of OpenMode, and passes 0 for Attributes. If >> + EFI_FILE_PROTOCOL.Open() fails, and OpenMode includes >> EFI_FILE_MODE_CREATE, >> + then the operation is retried with the caller's OpenMode and Attributes >> + unmodified. >> + >> +
Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
Thanks Laszlo. Would you please add one line comment on the FilePath, to describe if the FilePath is internal input or external input? As such the API consumer can know if caller’s responsibility to verify it or callee’s responsibility. For example, if the caller gets path from a read write variable, and input it directly, the this API need validate before use. If the caller already does the verification, then this API can use it directly. Sanity check is just for the format, not the content. The question I have is: Where should the sanity check be? thank you! Yao, Jiewen > 在 2018年7月19日，上午4:50，Laszlo Ersek 写道： > > The EfiOpenFileByDevicePath() function centralizes functionality from > > - MdeModulePkg/Universal/Disk/RamDiskDxe > - NetworkPkg/TlsAuthConfigDxe > - SecurityPkg/VariableAuthenticated/SecureBootConfigDxe > - ShellPkg/Library/UefiShellLib > > unifying the implementation and fixing various bugs. > > Cc: Chao Zhang > Cc: Eric Dong > Cc: Jaben Carsey > Cc: Jiaxin Wu > Cc: Jiewen Yao > Cc: Liming Gao > Cc: Michael D Kinney > Cc: Roman Bacik > Cc: Ruiyu Ni > Cc: Siyuan Fu > Cc: Star Zeng > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek > --- > MdePkg/Library/UefiLib/UefiLib.inf | 1 + > MdePkg/Include/Library/UefiLib.h | 86 > MdePkg/Library/UefiLib/UefiLib.c | 226 > 3 files changed, 313 insertions(+) > > diff --git a/MdePkg/Library/UefiLib/UefiLib.inf > b/MdePkg/Library/UefiLib/UefiLib.inf > index f69f0a43b576..a6c739ef3d6d 100644 > --- a/MdePkg/Library/UefiLib/UefiLib.inf > +++ b/MdePkg/Library/UefiLib/UefiLib.inf > @@ -68,6 +68,7 @@ [Protocols] > gEfiSimpleTextOutProtocolGuid ## SOMETIMES_CONSUMES > gEfiGraphicsOutputProtocolGuid ## SOMETIMES_CONSUMES > gEfiHiiFontProtocolGuid ## SOMETIMES_CONSUMES > + gEfiSimpleFileSystemProtocolGuid## SOMETIMES_CONSUMES > gEfiUgaDrawProtocolGuid | gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport > ## SOMETIMES_CONSUMES # Consumes if > gEfiGraphicsOutputProtocolGuid uninstalled > gEfiComponentNameProtocolGuid | NOT > gEfiMdePkgTokenSpaceGuid.PcdComponentNameDisable ## SOMETIMES_PRODUCES # > User chooses to produce it > gEfiComponentName2ProtocolGuid | NOT > gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable ## SOMETIMES_PRODUCES # > User chooses to produce it > diff --git a/MdePkg/Include/Library/UefiLib.h > b/MdePkg/Include/Library/UefiLib.h > index 7c6fde620c74..2468bf2aee80 100644 > --- a/MdePkg/Include/Library/UefiLib.h > +++ b/MdePkg/Include/Library/UefiLib.h > @@ -33,6 +33,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > #include > #include > #include > +#include > +#include > > #include > > @@ -1520,4 +1522,88 @@ EfiLocateProtocolBuffer ( > OUT UINTN *NoProtocols, > OUT VOID ***Buffer > ); > + > +/** > + Open or create a file or directory, possibly creating the chain of > + directories leading up to the directory. > + > + EfiOpenFileByDevicePath() first locates EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on > + FilePath, and opens the root directory of that filesystem with > + EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.OpenVolume(). > + > + On the remaining device path, the longest initial sequence of > + FILEPATH_DEVICE_PATH nodes is node-wise traversed with > + EFI_FILE_PROTOCOL.Open(). For the pathname fragment specified by each > + traversed FILEPATH_DEVICE_PATH node, EfiOpenFileByDevicePath() first masks > + EFI_FILE_MODE_CREATE out of OpenMode, and passes 0 for Attributes. If > + EFI_FILE_PROTOCOL.Open() fails, and OpenMode includes EFI_FILE_MODE_CREATE, > + then the operation is retried with the caller's OpenMode and Attributes > + unmodified. > + > + (As a consequence, if OpenMode includes EFI_FILE_MODE_CREATE, and > Attributes > + includes EFI_FILE_DIRECTORY, and each FILEPATH_DEVICE_PATH specifies a > single > + pathname component, then EfiOpenFileByDevicePath() ensures that the > specified > + series of subdirectories exist on return.) > + > + The EFI_FILE_PROTOCOL identified by the last FILEPATH_DEVICE_PATH node is > + output to the caller; intermediate EFI_FILE_PROTOCOL instances are closed. > If > + there are no FILEPATH_DEVICE_PATH nodes past the node that identifies the > + filesystem, then the EFI_FILE_PROTOCOL of the root directory of the > + filesystem is output to the caller. If a device path node that is different > + from FILEPATH_DEVICE_PATH is encountered relative to the filesystem, the > + traversal is stopped with an error, and a NULL EFI_FILE_PROTOCOL is output. > + > + @param[in,out] FilePath On input, the device path to the file or directory > + to open or create. On output, FilePath points one > + past the last node in the original
[edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
The EfiOpenFileByDevicePath() function centralizes functionality from - MdeModulePkg/Universal/Disk/RamDiskDxe - NetworkPkg/TlsAuthConfigDxe - SecurityPkg/VariableAuthenticated/SecureBootConfigDxe - ShellPkg/Library/UefiShellLib unifying the implementation and fixing various bugs. Cc: Chao Zhang Cc: Eric Dong Cc: Jaben Carsey Cc: Jiaxin Wu Cc: Jiewen Yao Cc: Liming Gao Cc: Michael D Kinney Cc: Roman Bacik Cc: Ruiyu Ni Cc: Siyuan Fu Cc: Star Zeng Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek --- MdePkg/Library/UefiLib/UefiLib.inf | 1 + MdePkg/Include/Library/UefiLib.h | 86 MdePkg/Library/UefiLib/UefiLib.c | 226 3 files changed, 313 insertions(+) diff --git a/MdePkg/Library/UefiLib/UefiLib.inf b/MdePkg/Library/UefiLib/UefiLib.inf index f69f0a43b576..a6c739ef3d6d 100644 --- a/MdePkg/Library/UefiLib/UefiLib.inf +++ b/MdePkg/Library/UefiLib/UefiLib.inf @@ -68,6 +68,7 @@ [Protocols] gEfiSimpleTextOutProtocolGuid ## SOMETIMES_CONSUMES gEfiGraphicsOutputProtocolGuid ## SOMETIMES_CONSUMES gEfiHiiFontProtocolGuid ## SOMETIMES_CONSUMES + gEfiSimpleFileSystemProtocolGuid## SOMETIMES_CONSUMES gEfiUgaDrawProtocolGuid | gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport ## SOMETIMES_CONSUMES # Consumes if gEfiGraphicsOutputProtocolGuid uninstalled gEfiComponentNameProtocolGuid | NOT gEfiMdePkgTokenSpaceGuid.PcdComponentNameDisable ## SOMETIMES_PRODUCES # User chooses to produce it gEfiComponentName2ProtocolGuid | NOT gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable ## SOMETIMES_PRODUCES # User chooses to produce it diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h index 7c6fde620c74..2468bf2aee80 100644 --- a/MdePkg/Include/Library/UefiLib.h +++ b/MdePkg/Include/Library/UefiLib.h @@ -33,6 +33,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include #include +#include +#include #include @@ -1520,4 +1522,88 @@ EfiLocateProtocolBuffer ( OUT UINTN *NoProtocols, OUT VOID ***Buffer ); + +/** + Open or create a file or directory, possibly creating the chain of + directories leading up to the directory. + + EfiOpenFileByDevicePath() first locates EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on + FilePath, and opens the root directory of that filesystem with + EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.OpenVolume(). + + On the remaining device path, the longest initial sequence of + FILEPATH_DEVICE_PATH nodes is node-wise traversed with + EFI_FILE_PROTOCOL.Open(). For the pathname fragment specified by each + traversed FILEPATH_DEVICE_PATH node, EfiOpenFileByDevicePath() first masks + EFI_FILE_MODE_CREATE out of OpenMode, and passes 0 for Attributes. If + EFI_FILE_PROTOCOL.Open() fails, and OpenMode includes EFI_FILE_MODE_CREATE, + then the operation is retried with the caller's OpenMode and Attributes + unmodified. + + (As a consequence, if OpenMode includes EFI_FILE_MODE_CREATE, and Attributes + includes EFI_FILE_DIRECTORY, and each FILEPATH_DEVICE_PATH specifies a single + pathname component, then EfiOpenFileByDevicePath() ensures that the specified + series of subdirectories exist on return.) + + The EFI_FILE_PROTOCOL identified by the last FILEPATH_DEVICE_PATH node is + output to the caller; intermediate EFI_FILE_PROTOCOL instances are closed. If + there are no FILEPATH_DEVICE_PATH nodes past the node that identifies the + filesystem, then the EFI_FILE_PROTOCOL of the root directory of the + filesystem is output to the caller. If a device path node that is different + from FILEPATH_DEVICE_PATH is encountered relative to the filesystem, the + traversal is stopped with an error, and a NULL EFI_FILE_PROTOCOL is output. + + @param[in,out] FilePath On input, the device path to the file or directory + to open or create. On output, FilePath points one + past the last node in the original device path that + has been successfully processed. FilePath is set on + output even if EfiOpenFileByDevicePath() returns an + error. + + @param[out] File On error, File is set to NULL. On success, File is + set to the EFI_FILE_PROTOCOL of the root directory + of the filesystem, if there are no + FILEPATH_DEVICE_PATH nodes in FilePath; otherwise, + File is set to the EFI_FILE_PROTOCOL identified by + the last node in FilePath. + + @param[in] OpenMode The OpenMode parameter to pass to + EFI_FILE_PROTOCOL.Open(). For each + FILEPATH_DEVICE_PATH node in
