Re: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Continue reading the patch #2, I think we can change IS_PD to:
#define IS_UDF_PD(Tag) ((Tag)->TagIdentifier == 5)

Using the above way, we can avoid caller to supply an invalid buffer.

Thanks/Ray

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ni,
> Ruiyu
> Sent: Monday, September 18, 2017 8:29 AM
> To: Paulo Alcantara <pcacjr@zytor.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [PATCH v2 1/3] MdePkg: Add UDF volume structure
> definitions
> 
> #define _GET_TAG_ID(_Pointer) \
>   (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
> 
> #define IS_PD(_Pointer) \
>   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
> #define IS_LVD(_Pointer) \
>   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
> #define IS_TD(_Pointer) \
>   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
> 
> #define IS_AVDP(_Pointer) \
>   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
> 
> Paulo,
> If you take a look at Pci22.h in the same directory, all macros are started
> as "IS_PCI_".
> How about changing the above IS_xxx to IS_UDF_xxx?
> Shall we change AVDP to AVD, following the same naming style as PD, LVD
> and TD?
> How about changing _Pointer to _Tag or DescriptorTag?
> 
> 
> Thanks/Ray
> 
> > -----Original Message-----
> > From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> > Sent: Sunday, September 17, 2017 9:13 PM
> > To: edk2-devel@lists.01.org
> > Cc: Paulo Alcantara <pcacjr@zytor.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
> Laszlo
> > Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> > Subject: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
> >
> > This patch adds a fewe more UDF structures in order to detect Logical
> > Volume and Partition descriptors during Main Volume Descriptor Sequence
> > in Partition driver.
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> > ---
> >  MdePkg/Include/IndustryStandard/Udf.h | 63 ++++++++++++++++++++
> >  1 file changed, 63 insertions(+)
> >
> > diff --git a/MdePkg/Include/IndustryStandard/Udf.h
> > b/MdePkg/Include/IndustryStandard/Udf.h
> > index 0febb4bcda..6cc42f8543 100644
> > --- a/MdePkg/Include/IndustryStandard/Udf.h
> > +++ b/MdePkg/Include/IndustryStandard/Udf.h
> > @@ -27,9 +27,19 @@
> >  #define _GET_TAG_ID(_Pointer) \
> >    (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
> >
> > +#define IS_PD(_Pointer) \
> > +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) #define IS_LVD(_Pointer) \
> > +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) #define IS_TD(_Pointer) \
> > +  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
> > +
> >  #define IS_AVDP(_Pointer) \
> >    ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
> >
> > +#define LV_UDF_REVISION(_Lv) \
> > +  *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
> > +
> >  #pragma pack(1)
> >
> >  typedef struct {
> > @@ -55,6 +65,59 @@ typedef struct {
> >    UINT8               Reserved[480];
> >  } UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;
> >
> > +typedef struct {
> > +  UINT8           CharacterSetType;
> > +  UINT8           CharacterSetInfo[63];
> > +} UDF_CHAR_SPEC;
> > +
> > +typedef struct {
> > +  UINT8           Flags;
> > +  UINT8           Identifier[23];
> > +  UINT8           IdentifierSuffix[8];
> > +} UDF_ENTITY_ID;
> > +
> > +typedef struct {
> > +  UINT32        LogicalBlockNumber;
> > +  UINT16        PartitionReferenceNumber;
> > +} UDF_LB_ADDR;
> > +
> > +typedef struct {
> > +  UINT32                           ExtentLength;
> > +  UDF_LB_ADDR                      ExtentLocation;
> > +  UINT8                            ImplementationUse[6];
> > +} UDF_LONG_ALLOCATION_DESCRIPTOR;
> > +
> > +typedef struct {
> > +  UDF_DESCRIPTOR_TAG              DescriptorTag;
> > +  UINT32                          VolumeDescriptorSequenceNumber;
> > +  UDF_CHAR_SPEC                   DescriptorCharacterSet;
> > +  UINT8                           LogicalVolumeIdentifier[128];
> > +  UINT32                          LogicalBlockSize;
> > +  UDF_ENTITY_ID                   DomainIdentifier;
> > +  UDF_LONG_ALLOCATION_DESCRIPTOR  LogicalVolumeContentsUse;
> > +  UINT32                          MapTableLength;
> > +  UINT32                          NumberOfPartitionMaps;
> > +  UDF_ENTITY_ID                   ImplementationIdentifier;
> > +  UINT8                           ImplementationUse[128];
> > +  UDF_EXTENT_AD                   IntegritySequenceExtent;
> > +  UINT8                           PartitionMaps[6];
> > +} UDF_LOGICAL_VOLUME_DESCRIPTOR;
> > +
> > +typedef struct {
> > +  UDF_DESCRIPTOR_TAG         DescriptorTag;
> > +  UINT32                     VolumeDescriptorSequenceNumber;
> > +  UINT16                     PartitionFlags;
> > +  UINT16                     PartitionNumber;
> > +  UDF_ENTITY_ID              PartitionContents;
> > +  UINT8                      PartitionContentsUse[128];
> > +  UINT32                     AccessType;
> > +  UINT32                     PartitionStartingLocation;
> > +  UINT32                     PartitionLength;
> > +  UDF_ENTITY_ID              ImplementationIdentifier;
> > +  UINT8                      ImplementationUse[128];
> > +  UINT8                      Reserved[156];
> > +} UDF_PARTITION_DESCRIPTOR;
> > +
> >  #pragma pack()
> >
> >  #endif
> > --
> > 2.11.0
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel