From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B17E71A1E3A for ; Mon, 24 Oct 2016 12:54:08 -0700 (PDT) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 34675C04B955; Mon, 24 Oct 2016 19:54:08 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-107.phx2.redhat.com [10.3.116.107]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9OJs6Fi004771; Mon, 24 Oct 2016 15:54:06 -0400 To: Jordan Justen , Ard Biesheuvel References: <20161021212737.15974-1-lersek@redhat.com> <20161021212737.15974-18-lersek@redhat.com> <147733485028.16732.10373366566840840474@jljusten-ivb> Cc: edk2-devel-01 , Michael Zimmermann , Leif Lindholm From: Laszlo Ersek Message-ID: <93732147-3d1e-bec6-0896-a7b286fd9795@redhat.com> Date: Mon, 24 Oct 2016 21:54:05 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <147733485028.16732.10373366566840840474@jljusten-ivb> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Mon, 24 Oct 2016 19:54:08 +0000 (UTC) Subject: Re: [PATCH 17/19] ArmPkg/ArmDisassemblerLib: replace AsciiStrCat() with AsciiStrCatS() 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: Mon, 24 Oct 2016 19:54:08 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 10/24/16 20:47, Jordan Justen wrote: > On 2016-10-24 00:58:46, Ard Biesheuvel wrote: >> On 21 October 2016 at 22:27, Laszlo Ersek wrote: >>> AsciiStrCat() is deprecated / disabled under the >>> DISABLE_NEW_DEPRECATED_INTERFACES feature test macro. >>> >>> The "Str" variable serves no particular purpose in the MRegList() and >>> ThumbMRegList() functions; replace it with the pointed-to "mMregListStr" / >>> "mThumbMregListStr" global variable (as appropriate), so that the new >>> AsciiStrCatS() calls are as clear as possible. >>> >>> Cc: Ard Biesheuvel >>> Cc: Leif Lindholm >>> Cc: Michael Zimmermann >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=164 >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=165 >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Laszlo Ersek >> >> Reviewed-by: Ard Biesheuvel >> >>> --- >>> >>> Notes: >>> - build-tested only >>> >>> - Michael (CC'd) had posted a patch earlier for this: >>> , >>> but I only noticed that now that he pointed it out, in >>> . >>> I'll leave it to the ArmPkg maintainers to choose one; I don't feel >>> strongly either way. >>> >> >> Apologies to Michael for ignoring his contribution, but I'd be happy >> for Laszlo to merge this as part of the series. >> >>> ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c | 22 +++++++++----------- >>> ArmPkg/Library/ArmDisassemblerLib/ThumbDisassembler.c | 20 ++++++++---------- >>> 2 files changed, 19 insertions(+), 23 deletions(-) >>> >>> diff --git a/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c b/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c >>> index 29a8d4438622..8551edc379c1 100644 >>> --- a/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c >>> +++ b/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c >>> @@ -88,12 +88,10 @@ MRegList ( >>> ) >>> { >>> UINTN Index, Start, End; >>> - CHAR8 *Str; >>> BOOLEAN First; >>> >>> - Str = mMregListStr; >>> - *Str = '\0'; >>> - AsciiStrCat (Str, "{"); >>> + mMregListStr[0] = '\0'; >>> + AsciiStrCatS (mMregListStr, sizeof mMregListStr, "{"); > > Maybe drop the extra space after the function name throughout this > patch. (There are 2 instead of just one.) Yep, I saw that; I couldn't decide whether to remove them or to leave them undisturbed. I will remove them. Thanks! Laszlo > > -Jordan > >>> for (Index = 0, First = TRUE; Index <= 15; Index++) { >>> if ((OpCode & (1 << Index)) != 0) { >>> Start = End = Index; >>> @@ -102,25 +100,25 @@ MRegList ( >>> } >>> >>> if (!First) { >>> - AsciiStrCat (Str, ","); >>> + AsciiStrCatS (mMregListStr, sizeof mMregListStr, ","); >>> } else { >>> First = FALSE; >>> } >>> >>> if (Start == End) { >>> - AsciiStrCat (Str, gReg[Start]); >>> - AsciiStrCat (Str, ", "); >>> + AsciiStrCatS (mMregListStr, sizeof mMregListStr, gReg[Start]); >>> + AsciiStrCatS (mMregListStr, sizeof mMregListStr, ", "); >>> } else { >>> - AsciiStrCat (Str, gReg[Start]); >>> - AsciiStrCat (Str, "-"); >>> - AsciiStrCat (Str, gReg[End]); >>> + AsciiStrCatS (mMregListStr, sizeof mMregListStr, gReg[Start]); >>> + AsciiStrCatS (mMregListStr, sizeof mMregListStr, "-"); >>> + AsciiStrCatS (mMregListStr, sizeof mMregListStr, gReg[End]); >>> } >>> } >>> } >>> if (First) { >>> - AsciiStrCat (Str, "ERROR"); >>> + AsciiStrCatS (mMregListStr, sizeof mMregListStr, "ERROR"); >>> } >>> - AsciiStrCat (Str, "}"); >>> + AsciiStrCatS (mMregListStr, sizeof mMregListStr, "}"); >>> >>> // BugBug: Make caller pass in buffer it is cleaner >>> return mMregListStr; >>> diff --git a/ArmPkg/Library/ArmDisassemblerLib/ThumbDisassembler.c b/ArmPkg/Library/ArmDisassemblerLib/ThumbDisassembler.c >>> index 5bad3afcfbf6..86d5083cb189 100644 >>> --- a/ArmPkg/Library/ArmDisassemblerLib/ThumbDisassembler.c >>> +++ b/ArmPkg/Library/ArmDisassemblerLib/ThumbDisassembler.c >>> @@ -397,12 +397,10 @@ ThumbMRegList ( >>> ) >>> { >>> UINTN Index, Start, End; >>> - CHAR8 *Str; >>> BOOLEAN First; >>> >>> - Str = mThumbMregListStr; >>> - *Str = '\0'; >>> - AsciiStrCat (Str, "{"); >>> + mThumbMregListStr[0] = '\0'; >>> + AsciiStrCatS (mThumbMregListStr, sizeof mThumbMregListStr, "{"); >>> >>> for (Index = 0, First = TRUE; Index <= 15; Index++) { >>> if ((RegBitMask & (1 << Index)) != 0) { >>> @@ -412,24 +410,24 @@ ThumbMRegList ( >>> } >>> >>> if (!First) { >>> - AsciiStrCat (Str, ","); >>> + AsciiStrCatS (mThumbMregListStr, sizeof mThumbMregListStr, ","); >>> } else { >>> First = FALSE; >>> } >>> >>> if (Start == End) { >>> - AsciiStrCat (Str, gReg[Start]); >>> + AsciiStrCatS (mThumbMregListStr, sizeof mThumbMregListStr, gReg[Start]); >>> } else { >>> - AsciiStrCat (Str, gReg[Start]); >>> - AsciiStrCat (Str, "-"); >>> - AsciiStrCat (Str, gReg[End]); >>> + AsciiStrCatS (mThumbMregListStr, sizeof mThumbMregListStr, gReg[Start]); >>> + AsciiStrCatS (mThumbMregListStr, sizeof mThumbMregListStr, "-"); >>> + AsciiStrCatS (mThumbMregListStr, sizeof mThumbMregListStr, gReg[End]); >>> } >>> } >>> } >>> if (First) { >>> - AsciiStrCat (Str, "ERROR"); >>> + AsciiStrCatS (mThumbMregListStr, sizeof mThumbMregListStr, "ERROR"); >>> } >>> - AsciiStrCat (Str, "}"); >>> + AsciiStrCatS (mThumbMregListStr, sizeof mThumbMregListStr, "}"); >>> >>> // BugBug: Make caller pass in buffer it is cleaner >>> return mThumbMregListStr; >>> -- >>> 2.9.2 >>> >>> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel