From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x231.google.com (mail-it0-x231.google.com [IPv6:2607:f8b0:4001:c0b::231]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E5E0C21E8798D for ; Fri, 8 Sep 2017 12:18:37 -0700 (PDT) Received: by mail-it0-x231.google.com with SMTP id 6so3761395itl.1 for ; Fri, 08 Sep 2017 12:21:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=VzG8KbtLK7Q2HdjaSRya8km6wdNWd6jxKT8W1hDpNYI=; b=TavpCkrbaoO/8MXWTPKwH71bs3iahU6wu3LEGL9YJBxGBBGhSgWNcg1MGKdia0ToKT +e+hc/nk1xCaPuk2jYroUHTq7Q6KbPb5DHhQHQ1PGh0xTlaSR4rQY12oYKpsEZTRToZb HHUAyGDmdOi9mLFVcNBwf/pHasNmrAcgyGb84= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=VzG8KbtLK7Q2HdjaSRya8km6wdNWd6jxKT8W1hDpNYI=; b=a1VPd1iGgfYXrFJ0Wbs8mkiU8f6M2P+R10IiJMTEqQjrJjCmfK81HjaCktFGQxicv+ ny4OL4kg9bvN6W7TOfWId90rrq/609dpDWCiJPJPFnpH2n6+5eqBz8a9PEOD76tbqI/B AkL+9uv0NdzBIaIiL8zWdXKCRZe1EZmymsBg7xSjzRvt5P+XEFSEh5eh1x4E6DeM1PyW SBCsdHbw80EJzJfwzMCxkUFGTkKY1QHz5siwdrPZYVdAyzdYxxhwPvxFLPIEwwmUJZL2 7ikVSQPfip+vdznR3RH2O+K2YoP9p7Krp3sQjrubxmiRkQvz8eGflYoKHy1yahRjPTIl oJfA== X-Gm-Message-State: AHPjjUilxbgsTb7SPROWoIIYIijkUoZV1IY9hr9obtP7J+ih908BAV3s Bfj61ahwNeLu0P+CBHK7ATVPAg16GzGK X-Google-Smtp-Source: AOwi7QBc8r65kZ9EH/WGpOAikwRDb9MpRMwh3MdEKBCO6jxLCvOdLtG+l2svouFXl44yIewi563h7nLK6/mR7Q1mg3o= X-Received: by 10.36.50.150 with SMTP id j144mr2474840ita.52.1504898489781; Fri, 08 Sep 2017 12:21:29 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.162.1 with HTTP; Fri, 8 Sep 2017 12:21:28 -0700 (PDT) In-Reply-To: <77914c33-7637-e759-02d0-f94e448157ff@redhat.com> References: <77914c33-7637-e759-02d0-f94e448157ff@redhat.com> From: Ard Biesheuvel Date: Fri, 8 Sep 2017 20:21:28 +0100 Message-ID: To: Laszlo Ersek Cc: Paulo Alcantara , "edk2-devel@lists.01.org" , Ruiyu Ni , Eric Dong , "Wu, Hao A" , Jordan Justen , Andrew Fish , Liming Gao , Michael D Kinney , Star Zeng Subject: Re: [PATCH v6 0/6] read-only UDF file system support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Sep 2017 19:18:38 -0000 Content-Type: text/plain; charset="UTF-8" On 8 September 2017 at 19:47, Laszlo Ersek wrote: > On 09/08/17 14:41, Paulo Alcantara wrote: >> Hi, >> >> This series introduces read-only UDF file system support in EDK2. As >> Laszlo (or Red Hat) seemed to be interested in such support, I'm posting >> it again after ~3 years. >> >> The idea is not replacing the default FAT file system, nor breaking any >> existing file system support, but extending EDK2 with a new file system >> that might be useful for some people who are looking for specific file >> system features that current FAT doesn't support. >> >> Originally the driver was written to support UDF file systems as >> specified by OSTA Universal Disk Format Specification 2.60. However, >> some Windows 10 Enterprise ISO (UDF bridge) images that I tested >> supported a revision of 1.02 thus I had to rework the driver a little >> bit to support such revision as well. >> >> v2: >> - Rework to _partially_ support UDF revisions <2.60. >> - Use existing CDROM_VOLUME_DESCRIPTOR structure defined in Eltorito.h >> instead of creating another one (UDF_VOLUME_DESCRIPTOR). >> - Fixed UdfDxe to correctly follow UEFI driver model. >> - Use HARDDRIVE_DEVICE_PATH instead of a vendor-defined one. >> - Detect UDF file systems only in PartitionDxe, and let UdfDxe driver >> check for specific UDF device path to decide whether or not install >> SimpleFs protocol. >> - Place MdePkg changes in a separate patch. >> v3: >> - Install UDF partition child handles with a Vendor-Defined Media >> Device Path. >> - Changed UdfDxe to check for Vendor-Defined Media Device Paths with a >> specific UDF file system GUID when determining to whether or not >> start the driver. >> - Removed leading TAB chars in some source files identified by >> PatchCheck.py tool. >> v4: >> - Added missing R-b's. >> v5: >> - Fixed OVMF IA32 build. >> - Fixed a typo in UdfDriveBindingStop() ("This" -> "SimpleFs") which >> broke retrieval of private fs data from SimpleFs protocol -- >> identified by 'reconnect -r' command in UEFI shell. >> v6: >> - Fixed a bug in UdfRead() that'd pontentially break in ARM or IA32 >> by allowing caller to read more than 4GiB of data >> (i.e. BufferSize pointer is dereferenced as an UINT64 * and it's >> followed by 4 bytes that are nonzero). >> >> Repo: https://github.com/pcacjr/edk2.git >> Branch: udf-fs-v6 >> >> Cc: Laszlo Ersek >> Cc: Jordan Justen >> Cc: Andrew Fish >> Cc: Michael D Kinney >> Cc: Liming Gao >> Cc: Star Zeng >> Cc: Eric Dong >> Cc: Mark Doran >> Cc: Ruiyu Ni >> Cc: hao.a.wu@intel.com >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Paulo Alcantara >> --- >> >> Paulo Alcantara (6): >> MdePkg: Add UDF volume structure definitions >> MdeModulePkg/PartitionDxe: Add UDF file system support >> MdeModulePkg: Initial UDF/ECMA-167 file system support >> OvmfPkg: Enable UDF file system support >> ArmVirtPkg: Enable UDF file system support >> Nt32Pkg: Enable UDF file system support >> >> ArmVirtPkg/ArmVirtQemu.dsc | 3 +- >> ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 3 +- >> ArmVirtPkg/ArmVirtQemuKernel.dsc | 3 +- >> ArmVirtPkg/ArmVirtXen.dsc | 3 +- >> ArmVirtPkg/ArmVirtXen.fdf | 1 + >> .../Universal/Disk/PartitionDxe/Partition.c | 9 +- >> .../Universal/Disk/PartitionDxe/Partition.h | 32 +- >> .../Universal/Disk/PartitionDxe/PartitionDxe.inf | 3 +- >> MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 318 +++ >> MdeModulePkg/Universal/Disk/UdfDxe/ComponentName.c | 185 ++ >> MdeModulePkg/Universal/Disk/UdfDxe/File.c | 908 ++++++++ >> MdeModulePkg/Universal/Disk/UdfDxe/FileName.c | 195 ++ >> .../Universal/Disk/UdfDxe/FileSystemOperations.c | 2447 ++++++++++++++++++++ >> MdeModulePkg/Universal/Disk/UdfDxe/Udf.c | 344 +++ >> MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 1244 ++++++++++ >> MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf | 66 + >> MdePkg/Include/IndustryStandard/Udf.h | 60 + >> Nt32Pkg/Nt32Pkg.dsc | 1 + >> Nt32Pkg/Nt32Pkg.fdf | 1 + >> OvmfPkg/OvmfPkgIa32.dsc | 1 + >> OvmfPkg/OvmfPkgIa32.fdf | 1 + >> OvmfPkg/OvmfPkgIa32X64.dsc | 1 + >> OvmfPkg/OvmfPkgIa32X64.fdf | 1 + >> OvmfPkg/OvmfPkgX64.dsc | 1 + >> OvmfPkg/OvmfPkgX64.fdf | 1 + >> 25 files changed, 5821 insertions(+), 11 deletions(-) >> create mode 100644 MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c >> create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/ComponentName.c >> create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/File.c >> create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/FileName.c >> create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c >> create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/Udf.c >> create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/Udf.h >> create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf >> create mode 100644 MdePkg/Include/IndustryStandard/Udf.h >> > > Pushed as commit range 7aee391fa3d0..b696c64d4fc3. > This code breaks the Clang build: :937:11: error: variable 'Status' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] if (ReadFileInfo->FileData == NULL) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ :1148:10: note: uninitialized use occurs here return Status; ^~~~~~ :937:7: note: remove the 'if' if its condition is always true if (ReadFileInfo->FileData == NULL) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ :932:16: error: variable 'Status' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] } else if (ReadFileInfo->Flags == READ_FILE_ALLOCATE_AND_READ) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ :1148:10: note: uninitialized use occurs here return Status; ^~~~~~ :932:12: note: remove the 'if' if its condition is always true } else if (ReadFileInfo->Flags == READ_FILE_ALLOCATE_AND_READ) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ :930:9: error: variable 'Status' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized] if (ReadFileInfo->Flags == READ_FILE_GET_FILESIZE) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ :1148:10: note: uninitialized use occurs here return Status; ^~~~~~ :930:5: note: remove the 'if' if its condition is always false if (ReadFileInfo->Flags == READ_FILE_GET_FILESIZE) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ :869:33: note: initialize the variable 'Status' to silence this warning EFI_STATUS Status; ^ = 0 3 errors generated. Thanks, Ard.