(heh - forgot to answer your question about the GUID :-) )
On 8/9/2017 10:26 AM, Paulo Alcantara wrote:
Hi Ray,
Thanks for the review. My comments below.
On 8/9/2017 3:05 AM, Ni, Ruiyu wrote:
Paulo,
Thanks for enabling the UDF support into EDKII.
Below are several comments:
1. Could you please separate the patch to modify MdePkg and MdeModulePkg?
Sure.
2. UDF_CDROM_VOLUME_IDENTIFIER is also defined in Eltorito.h as CDVOL_ID.
Maybe we could redefine CDVOL_ID to UDF_CDROM_VOLUME_IDENTIFIER?
The Volume Identifier structure is defined in ECMA-167 specification
(not UDF specific), so perhaps it would be better if we create a
separate header file (e.g., Ecma-167.h) and define
ECMA167_VOLUME_IDENTIFIER structure. That would be used in both ElTorito
and UDF codes. What do you think?
2. Why do you need a PCD to control the UDF support? I prefer no. More
choices
is not always good😊
The original idea of including this PCD was to avoid adding unnecessary
overhead in PartitionDxe driver to something (UDF) that wasn't actually
defined in UEFI specification -- leaving it as an optional feature.
But yes, I agree with you that just complicates things and would be
better to remove it.
3. Is gUdfVolumeSignatureGuid only used in Partition driver to
produce the device
path? Can we just use HARDDRIVE_DEVICE_PATH? Or at least
gUdfVolumeSignatureGuid
can be removed, the GUID macro is enough?
The GUID is used in both Partition and UdfDxe drivers. Do you mean that
we should use HARDDRIVE_DEVICE_PATH and then appending the UDF-specific
GUID to it?
Thanks,
Paulo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
