From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <ard.biesheuvel@linaro.org>
Received: from mail-io0-x22e.google.com (mail-io0-x22e.google.com
 [IPv6:2607:f8b0:4001:c06::22e])
 (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 6889A21A1349C
 for <edk2-devel@lists.01.org>; Thu, 18 May 2017 08:13:00 -0700 (PDT)
Received: by mail-io0-x22e.google.com with SMTP id k91so30479341ioi.1
 for <edk2-devel@lists.01.org>; Thu, 18 May 2017 08:13:00 -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=7zkSAzMdq3wJ3Mhm+eoBPjUjQQ7q4ebPy6LZGq9Qx78=;
 b=GIDFp9fubkhlxg7dmaJ/R0t8B3C4reD+KUp6aARsbgA8o7zAKKqwAtcr6EBLO7v4RT
 1CD18TxCYm9UmzButT7x0ATPJ//ovU5Y29WVRKjOKCBHUTbU2gtVezEnHXhODI+q3le9
 a0r+0klRo6VMLPjbk39e41KUdYqItUORQDfIM=
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=7zkSAzMdq3wJ3Mhm+eoBPjUjQQ7q4ebPy6LZGq9Qx78=;
 b=HHdeU+Xs6oijRc/ddM3sWYyOAeVG92VXWDjCpeOt9896RGG1cNq5wVVI/L9IdyA66I
 LT8Axi61kg6VDonaDrod5Lv3Sdt4WSAFC9CUFoDdw9+YVcjCj/zVGnfvTb8mjiTgsfuR
 8qhGh6bRKOD3eWe6PpaeM4ReYu86ZSZh8IS2lMDvoB2R/w2aRNzx4Xsp7w8eXw8fa0FJ
 wKHB9R1dR3brtVym/gHpd41xBUpQKVnAV7mzuZCV//VXoftD9DMoFmVYm8FbJwvprFL8
 RJ/8MeAL+faOwEXMMiOXEU3ky8ZDeX2pHB1eGNO/tNPxvXqk0tnYgCHkKsJjwtPCjIoS
 Z3rQ==
X-Gm-Message-State: AODbwcDr1k7cdZMvZbKq9UAxBc0evZzjgxD+xENax4OF8joRmF3Xznk1
 d22TdWgmlvGEba0TI2TnoSiYrp6WeDSz
X-Received: by 10.107.26.144 with SMTP id a138mr4647996ioa.72.1495120379639;
 Thu, 18 May 2017 08:12:59 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.107.164.24 with HTTP; Thu, 18 May 2017 08:12:59 -0700 (PDT)
In-Reply-To: <20170518150427.16435-2-lersek@redhat.com>
References: <20170518150427.16435-1-lersek@redhat.com>
 <20170518150427.16435-2-lersek@redhat.com>
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Thu, 18 May 2017 16:12:59 +0100
Message-ID: <CAKv+Gu-AkBcCe5ABErDcC5ZFZ6m+=cBcDguXrdkszTFxt-tMsg@mail.gmail.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>,
 Jordan Justen <jordan.l.justen@intel.com>, 
 Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [PATCH 1/8] ArmPlatformPkg/NorFlashDxe: correct NumOfLba vararg type in EraseBlocks()
X-BeenThere: edk2-devel@lists.01.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: EDK II Development  <edk2-devel.lists.01.org>
List-Unsubscribe: <https://lists.01.org/mailman/options/edk2-devel>,
 <mailto:edk2-devel-request@lists.01.org?subject=unsubscribe>
List-Archive: <http://lists.01.org/pipermail/edk2-devel/>
List-Post: <mailto:edk2-devel@lists.01.org>
List-Help: <mailto:edk2-devel-request@lists.01.org?subject=help>
List-Subscribe: <https://lists.01.org/mailman/listinfo/edk2-devel>,
 <mailto:edk2-devel-request@lists.01.org?subject=subscribe>
X-List-Received-Date: Thu, 18 May 2017 15:13:00 -0000
Content-Type: text/plain; charset="UTF-8"

On 18 May 2017 at 16:04, Laszlo Ersek <lersek@redhat.com> wrote:
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
>
>> The variable argument list is a list of tuples. Each tuple describes a
>> range of LBAs to erase and consists of the following:
>> * An EFI_LBA that indicates the starting LBA
>> * A UINTN that indicates the number of blocks to erase
>
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
>
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
>
> In addition, update the DEBUG macro invocation where NumOfLba is formatted
> with the %d conversion specifier: UINTN values should be converted to
> UINT64 and printed with %Lu or %Lx for portability between 32-bit and
> 64-bit.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> index 12a861267a86..3ea858f46ffb 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> @@ -628,10 +628,16 @@ FvbEraseBlocks (
>      }
>
>      // How many Lba blocks are we requested to erase?
> -    NumOfLba = VA_ARG (Args, UINT32);
> +    NumOfLba = VA_ARG (Args, UINTN);
>
>      // All blocks must be within range
> -    DEBUG ((DEBUG_BLKIO, "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%d - 1 ) > LastBlock=%ld.\n", Instance->StartLba + StartingLba, NumOfLba, Instance->Media.LastBlock));
> +    DEBUG ((
> +      DEBUG_BLKIO,
> +      "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) > LastBlock=%ld.\n",
> +      Instance->StartLba + StartingLba,
> +      (UINT64)NumOfLba,
> +      Instance->Media.LastBlock
> +      ));
>      if ((NumOfLba == 0) || ((Instance->StartLba + StartingLba + NumOfLba - 1) > Instance->Media.LastBlock)) {
>        VA_END (Args);
>        DEBUG ((EFI_D_ERROR, "FvbEraseBlocks: ERROR - Lba range goes past the last Lba.\n"));
> @@ -656,7 +662,7 @@ FvbEraseBlocks (
>      }
>
>      // How many Lba blocks are we requested to erase?
> -    NumOfLba = VA_ARG (Args, UINT32);
> +    NumOfLba = VA_ARG (Args, UINTN);
>
>      // Go through each one and erase it
>      while (NumOfLba > 0) {
> --
> 2.9.3
>
>