From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x231.google.com (mail-wm0-x231.google.com [IPv6:2a00:1450:400c:c09::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 EF20181DBF for ; Fri, 28 Oct 2016 08:26:21 -0700 (PDT) Received: by mail-wm0-x231.google.com with SMTP id e69so109128006wmg.0 for ; Fri, 28 Oct 2016 08:26:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Qmtmq5hs/x1Rh1rqt7ZCqBLg4T3UL6dO7WAYXxFMZio=; b=iWfvC2zhJhu0Rpj6pPMq6GzZatV7TIBn4yaiSRqrwcbyNKKwR2r9CDCKMPcHioJYiT FvVaJ88ATQlfngyTtmTwMrZZc8JqvYDIjsmas4YlPme84xvosFsOAomklhPBXPq1w/rr eZ2RDWnnt8eHMFyGUvZQR59y9zIMeyW5bLOto= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Qmtmq5hs/x1Rh1rqt7ZCqBLg4T3UL6dO7WAYXxFMZio=; b=AqmWkRPgls8k+Fa/UjGgqQodW0KLj3U5b9PEZE6/pJYTllpoOJDlR32Z33VWZaIraL vUTcnFBW0XqICFg8az3E0dC9MkkqAo9CR6umreb8hIgXPEhLbvRe3n19iSIXojB7YLda FA4eN6jKqM9zSVmFtBydtmJBevYLULNVtmmPbn5JDW/HtDsjrTn78PuzzEBBOkIJkFG1 RwqN6apnhXIDbiZRMD+HQI5Vl3TGllGSVaZZhylHPcXQNq5yrGMWa2lApS7zxCb0L+Xc IuZNcKpMPsbxPpp8tU++Q0Zb/rVas2yPdlOtnXIEq25SpSUWpmFlbb2eLTwrIYFLagRF IXYQ== X-Gm-Message-State: ABUngvdkRFfMPE3g3FpVyHEaNJ7OxBzF2ZQpSyZ02DcmBIp+6vTR1XZEAz1c3KRWj7ndI1KF X-Received: by 10.28.31.147 with SMTP id f141mr3645297wmf.108.1477668381044; Fri, 28 Oct 2016 08:26:21 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id vf8sm14645178wjc.27.2016.10.28.08.26.20 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 28 Oct 2016 08:26:20 -0700 (PDT) Date: Fri, 28 Oct 2016 16:26:18 +0100 From: Leif Lindholm To: Ard Biesheuvel Cc: edk2-devel-01 , Laszlo Ersek , Ryan Harkin Message-ID: <20161028152618.GE1161@bivouac.eciton.net> References: <1477651721-16958-1-git-send-email-ard.biesheuvel@linaro.org> <1477651721-16958-2-git-send-email-ard.biesheuvel@linaro.org> <20161028144834.GU1161@bivouac.eciton.net> <20161028151340.GW1161@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [PATCH v2 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: Fri, 28 Oct 2016 15:26:22 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Oct 28, 2016 at 04:14:43PM +0100, Ard Biesheuvel wrote: > On 28 October 2016 at 16:13, Leif Lindholm wrote: > > On Fri, Oct 28, 2016 at 04:02:21PM +0100, Ard Biesheuvel wrote: > >> On 28 October 2016 at 15:48, Leif Lindholm wrote: > >> > On Fri, Oct 28, 2016 at 11:48:40AM +0100, 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 | 9 +++++---- > >> >> 1 file changed, 5 insertions(+), 4 deletions(-) > >> >> > >> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressFastBootDxe/ArmVExpressFastBoot.c > >> >> index 4d0811cc5eaf..64b25f8a8c45 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 > >> >> @@ -320,7 +320,8 @@ ArmFastbootPlatformFlashPartition ( > >> >> CHAR16 PartitionNameUnicode[60]; > >> >> BOOLEAN PartitionFound; > >> >> > >> >> - AsciiStrToUnicodeStr (PartitionName, PartitionNameUnicode); > >> >> + AsciiStrToUnicodeStrS (PartitionName, PartitionNameUnicode, > >> >> + ARRAY_SIZE (PartitionNameUnicode)); > >> >> > >> >> PartitionFound = FALSE; > >> >> Entry = (FASTBOOT_PARTITION_LIST *) GetFirstNode (&(mPartitionListHead)); > >> >> @@ -396,7 +397,7 @@ ArmFastbootPlatformGetVar ( > >> >> ) > >> >> { > >> >> if (AsciiStrCmp (Name, "product")) { > >> >> - AsciiStrCpy (Value, FixedPcdGetPtr (PcdFirmwareVendor)); > >> >> + AsciiStrCpyS (Value, 61, FixedPcdGetPtr (PcdFirmwareVendor)); > >> > > >> > I'm totally OK with the reason for hard-coding this, but could you add > >> > the comment from the feedback on previous version?: > >> > // FASTBOOT_COMMAND_MAX_LENGTH - 4 + NULL terminator > >> > (Or if there's a better way of putting it.) > >> > > >> > >> What if I decorate each entry point with the comment block from the > >> associated protocol header file instead? (in a follow up patch) > >> That is common practice in Tianocore anyway, and looks like this for > >> this particular protocol method: > >> > >> /* > >> If the variable referred to by Name exists, copy it (as a null-terminated > >> string) into Value. If it doesn't exist, put the Empty string in Value. > >> > >> Variable names and values may not be larger than 60 bytes, excluding the > >> terminal null character. This is a limitation of the Fastboot protocol. > >> > >> The Fastboot application will handle platform-nonspecific variables > >> (Currently "version" is the only one of these.) > >> > >> @param[in] Name Null-terminated name of Fastboot variable to retrieve. > >> @param[out] Value Caller-allocated buffer for null-terminated value of > >> variable. > >> > >> @retval EFI_SUCCESS The variable was retrieved, or it doesn't exist. > >> @retval EFI_DEVICE_ERROR There was an error looking up the variable. This > >> does _not_ include the variable not existing. > >> */ > > > > That works for me. > > > > Good. Does that mean you are happy with this patch as-is then? If it goes after the one I just acked, sure: Reviewed-by: Leif Lindholm