Re: [edk2-platforms][PATCH v2 2/2] Ext4Pkg: Add base containing record macro for EXT4_FILE

["Marvin Häuser" <mhaeuser@posteo.de>]

Reviewed-by: Marvin Häuser <mhaeuser@posteo.de>

> On 20. Jul 2022, at 08:47, Savva Mitrofanov <savvamtr@gmail.com> wrote:
> 
> We shouldn't use direct casts, because in the future it could break
> the code, so using BASE_CR would be safe against possible structure
> changes and rearrangements
> 
> Cc: Marvin Häuser <mhaeuser@posteo.de>
> Cc: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
> Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
> ---
> Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h |  2 ++
> Features/Ext4Pkg/Ext4Dxe/File.c    | 16 ++++++++--------
> 2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> index a1eb32aa2cff..3ff7b29e7133 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> @@ -383,6 +383,8 @@ struct _Ext4File {
>   EXT4_DENTRY           *Dentry;
> };
> 
> +#define EXT4_FILE_FROM_THIS(This)  BASE_CR ((This), EXT4_FILE, Protocol)
> +
> #define EXT4_FILE_FROM_OPEN_FILES_NODE(Node)                                   \
>   BASE_CR(Node, EXT4_FILE, OpenFilesListNode)
> 
> diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c b/Features/Ext4Pkg/Ext4Dxe/File.c
> index 0fb9e05e6647..750397010443 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/File.c
> +++ b/Features/Ext4Pkg/Ext4Dxe/File.c
> @@ -568,7 +568,7 @@ Ext4Open (
>   EXT4_FILE   *FoundFile;
>   EXT4_FILE   *Source;
> 
> -  Source = (EXT4_FILE *)This;
> +  Source = EXT4_FILE_FROM_THIS (This);
> 
>   //
>   // Reset Symloops counter
> @@ -605,7 +605,7 @@ Ext4Close (
>   IN EFI_FILE_PROTOCOL  *This
>   )
> {
> -  return Ext4CloseInternal ((EXT4_FILE *)This);
> +  return Ext4CloseInternal (EXT4_FILE_FROM_THIS (This));
> }
> 
> /**
> @@ -686,7 +686,7 @@ Ext4ReadFile (
>   EXT4_PARTITION  *Partition;
>   EFI_STATUS      Status;
> 
> -  File      = (EXT4_FILE *)This;
> +  File      = EXT4_FILE_FROM_THIS (This);
>   Partition = File->Partition;
> 
>   ASSERT (Ext4FileIsOpenable (File));
> @@ -737,7 +737,7 @@ Ext4WriteFile (
> {
>   EXT4_FILE  *File;
> 
> -  File = (EXT4_FILE *)This;
> +  File = EXT4_FILE_FROM_THIS (This);
> 
>   if (!(File->OpenMode & EFI_FILE_MODE_WRITE)) {
>     return EFI_ACCESS_DENIED;
> @@ -767,7 +767,7 @@ Ext4GetPosition (
> {
>   EXT4_FILE  *File;
> 
> -  File = (EXT4_FILE *)This;
> +  File = EXT4_FILE_FROM_THIS (This);
> 
>   if (Ext4FileIsDir (File)) {
>     return EFI_UNSUPPORTED;
> @@ -800,7 +800,7 @@ Ext4SetPosition (
> {
>   EXT4_FILE  *File;
> 
> -  File = (EXT4_FILE *)This;
> +  File = EXT4_FILE_FROM_THIS (This);
> 
>   // Only seeks to 0 (so it resets the ReadDir operation) are allowed
>   if (Ext4FileIsDir (File) && (Position != 0)) {
> @@ -1068,7 +1068,7 @@ Ext4GetInfo (
>   EXT4_FILE       *File;
>   EXT4_PARTITION  *Partition;
> 
> -  File      = (EXT4_FILE *)This;
> +  File      = EXT4_FILE_FROM_THIS (This);
>   Partition = File->Partition;
> 
>   if (CompareGuid (InformationType, &gEfiFileInfoGuid)) {
> @@ -1185,7 +1185,7 @@ Ext4SetInfo (
>   EXT4_FILE       *File;
>   EXT4_PARTITION  *Partition;
> 
> -  File      = (EXT4_FILE *)This;
> +  File      = EXT4_FILE_FROM_THIS (This);
>   Partition = File->Partition;
> 
>   if (Partition->ReadOnly) {
> -- 
> 2.37.0
>