From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x22d.google.com (mail-it0-x22d.google.com [IPv6:2607:f8b0:4001:c0b::22d]) (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 D61101A1E34 for ; Wed, 26 Oct 2016 03:34:56 -0700 (PDT) Received: by mail-it0-x22d.google.com with SMTP id e187so17927324itc.0 for ; Wed, 26 Oct 2016 03:34:56 -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=EoOtRRiglGp0DvPuf8Dxkmatw/IpQKcIKKwfmpbN2CE=; b=Cby0m+qRwk3/r+MEoQCgSxTlOWF4S2dWayqiKqQ0bH5NFi9YrbHSdUYYKE8Yn/sJk0 wIAkYn/CGa1DwVV0wEhfZyRy28JFc14Js3gTlVIbqsnu0tRU8zpdOX31NnSm0qhWMFqW pGemdZO394TV6Tx1R9mLOm/pXFEg+WfXcJfVM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=EoOtRRiglGp0DvPuf8Dxkmatw/IpQKcIKKwfmpbN2CE=; b=WWjz4PHuVgmOS4KHSy+atLQ1P9xjC1Lq7H2J1Sw/hPX+yLZGwel83PS6yZE252c/YY K0roSSH7EAm5XuiWJsI07P2LuFq6SY0nQHAr3mYdTtds97yr3MtrD1kkxXehNm2RmrkO Q8rQFr901S+r2m7Ir5Yx7ZhiF5hvIeByoZGT7ok5qCzoIscLGnOwuS7DfCkdK7c0xa36 xkiYCtDkrTlKgK+YCBj501LZuTwLA2hL/5vJ162Sfa/RRK9GDs8kJS7lrgCl1ae7Wh2b Afog9y6KU8MBAZ48jEPM/8H221eyp6dJmcM4O5GXyMbC61ZMObsj84PtQgv8DR9jiEtT gL2A== X-Gm-Message-State: ABUngvfu3rKYQDIBf9UlsQL+7tealjLpTu8kw4tlHCjR/z0gKMSnn1Mdl93AqA/4MuPf1SN9OKFHMPdMjube+pjZ X-Received: by 10.36.66.142 with SMTP id i136mr1445736itb.63.1477478095973; Wed, 26 Oct 2016 03:34:55 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.5.139 with HTTP; Wed, 26 Oct 2016 03:34:55 -0700 (PDT) In-Reply-To: <38b82d4d-3d07-a05a-6395-7e769f87e972@redhat.com> References: <1477419424-22235-1-git-send-email-ard.biesheuvel@linaro.org> <1477419424-22235-2-git-send-email-ard.biesheuvel@linaro.org> <38b82d4d-3d07-a05a-6395-7e769f87e972@redhat.com> From: Ard Biesheuvel Date: Wed, 26 Oct 2016 11:34:55 +0100 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Leif Lindholm Subject: Re: [PATCH 1/2] ArmPlatformPkg/ArmVExpressFastBootDxe: eliminate deprecated string functions X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Oct 2016 10:34:57 -0000 Content-Type: text/plain; charset=UTF-8 On 26 October 2016 at 11:32, Laszlo Ersek wrote: > On 10/25/16 20:17, Ard Biesheuvel wrote: >> Get rid of functions that are no longer available when defining >> DISABLE_NEW_DEPRECATED_INTERFACES >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel >> --- >> ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c >> index 4d0811cc5eaf..6b39682948aa 100644 >> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c >> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c >> @@ -269,7 +269,7 @@ ArmFastbootPlatformInit ( >> >> // Copy handle and partition name >> Entry->PartitionHandle = AllHandles[LoopIndex]; >> - StrnCpy ( >> + CopyMem ( >> Entry->PartitionName, >> PartitionEntries[PartitionNode->PartitionNumber - 1].PartitionName, // Partition numbers start from 1. >> PARTITION_NAME_MAX_LENGTH > > okay > >> @@ -320,7 +320,7 @@ ArmFastbootPlatformFlashPartition ( >> CHAR16 PartitionNameUnicode[60]; >> BOOLEAN PartitionFound; >> >> - AsciiStrToUnicodeStr (PartitionName, PartitionNameUnicode); >> + AsciiStrToUnicodeStrS (PartitionName, PartitionNameUnicode, 60); >> >> PartitionFound = FALSE; >> Entry = (FASTBOOT_PARTITION_LIST *) GetFirstNode (&(mPartitionListHead)); > > You asked me to introduce a macro for a very similar case in one of my > ArmPkg patches... > You are right, my apologies. In my defense, ArmPkg is something we consider maintained, whereas ArmPlatformPkg is a collection of cruft which we would like to phase out as soon as we can. > Anyway, the change is valid. > >> @@ -396,7 +396,7 @@ ArmFastbootPlatformGetVar ( >> ) >> { >> if (AsciiStrCmp (Name, "product")) { >> - AsciiStrCpy (Value, FixedPcdGetPtr (PcdFirmwareVendor)); >> + AsciiStrCpyS (Value, 60, FixedPcdGetPtr (PcdFirmwareVendor)); >> } else { >> *Value = '\0'; >> } > > This is wrong. > > The signature of this function does not indicate the expected size of > the receiving buffer. However, the function is a > FASTBOOT_PLATFORM_GETVAR implementation (== > FASTBOOT_PLATFORM_PROTOCOL.GetVar() member implementation). The leading > comment on that function pointer type says, > > Variable names and values may not be larger than 60 bytes, excluding the > terminal null character. This is a limitation of the Fastboot protocol. > > I.e., 60 non-NUL bytes, plus the terminating NUL, is a valid variable > value. Therefore the DestMax parameter should be 61. > > Just to be sure, I checked the call sites. There is only one call site > actually, in > "EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c", function > HandleGetVar(): > > CHAR8 Response[FASTBOOT_COMMAND_MAX_LENGTH + 1] = "OKAY"; > ... > Status = mPlatform->GetVar (CmdArg, Response + 4); > > FASTBOOT_COMMAND_MAX_LENGTH is 64 (same file), therefore (Response + 4) > points to a (sub-)array of 61 characters. IOW, the call site is > consistent with the protocol definition, and the DestMax param should be > bumped to 61. > Thanks for spotting that. >> @@ -410,7 +410,7 @@ ArmFastbootPlatformOemCommand ( >> { >> CHAR16 CommandUnicode[65]; >> >> - AsciiStrToUnicodeStr (Command, CommandUnicode); >> + AsciiStrToUnicodeStrS (Command, CommandUnicode, 65); >> >> if (AsciiStrCmp (Command, "Demonstrate") == 0) { >> DEBUG ((EFI_D_ERROR, "ARM OEM Fastboot command 'Demonstrate' received.\n")); >> > > This is correct. > Thanks, Ard.