From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org 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 2E12F21194891 for ; Wed, 5 Dec 2018 04:16:18 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 11361750D7; Wed, 5 Dec 2018 12:16:17 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-95.rdu2.redhat.com [10.10.120.95]) by smtp.corp.redhat.com (Postfix) with ESMTP id 836FC5C21C; Wed, 5 Dec 2018 12:16:15 +0000 (UTC) To: Ard Biesheuvel , edk2-devel@lists.01.org Cc: Bob Feng , Liming Gao , Yonghong Zhu References: <20181205082401.20224-1-ard.biesheuvel@linaro.org> From: Laszlo Ersek Message-ID: <5c6d9efc-cc93-e049-78c5-52bc78c709eb@redhat.com> Date: Wed, 5 Dec 2018 13:16:09 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20181205082401.20224-1-ard.biesheuvel@linaro.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Wed, 05 Dec 2018 12:16:17 +0000 (UTC) Subject: Re: [PATCH] BaseTools/CommonLib: drop the use of MAX_ADDRESS X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Dec 2018 12:16:18 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 12/05/18 09:24, Ard Biesheuvel wrote: > The macro MAX_ADDRESS represents the largest virtual address that > is valid for a certain architecture. For the BaseTools, this quantity > is irrelevant, since the same tools can be used to build for different > targets. > > Since we only refer to it in a single place, which is an ASSERT() that > doesn't seem particularly useful (it ensures that memcpy() will not > be called with arguments that will make it read beyond the end of the > address space and wrap around), let's drop the ASSERT and all references > to MAX_ADDRESS. > > Cc: Bob Feng > Cc: Liming Gao > Cc: Yonghong Zhu > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > BaseTools/Source/C/Include/AArch64/ProcessorBind.h | 5 ----- > BaseTools/Source/C/Include/Arm/ProcessorBind.h | 5 ----- > BaseTools/Source/C/Include/Common/UefiBaseTypes.h | 1 - > BaseTools/Source/C/Include/Ia32/ProcessorBind.h | 5 ----- > BaseTools/Source/C/Include/X64/ProcessorBind.h | 5 ----- > BaseTools/Source/C/Common/CommonLib.c | 1 - > 6 files changed, 22 deletions(-) > > diff --git a/BaseTools/Source/C/Include/AArch64/ProcessorBind.h b/BaseTools/Source/C/Include/AArch64/ProcessorBind.h > index e7e9d83198a6..f956cab453f0 100644 > --- a/BaseTools/Source/C/Include/AArch64/ProcessorBind.h > +++ b/BaseTools/Source/C/Include/AArch64/ProcessorBind.h > @@ -90,11 +90,6 @@ typedef INT64 INTN; > /// > #define MAX_2_BITS 0xC000000000000000 > > -/// > -/// Maximum legal AARCH64 address > -/// > -#define MAX_ADDRESS 0xFFFFFFFFFFFFFFFF > - > /// > /// The stack alignment required for AARCH64 > /// > diff --git a/BaseTools/Source/C/Include/Arm/ProcessorBind.h b/BaseTools/Source/C/Include/Arm/ProcessorBind.h > index be4aac97664d..856d2bd9eff7 100644 > --- a/BaseTools/Source/C/Include/Arm/ProcessorBind.h > +++ b/BaseTools/Source/C/Include/Arm/ProcessorBind.h > @@ -88,11 +88,6 @@ typedef INT32 INTN; > /// > #define MAX_2_BITS 0xC0000000 > > -/// > -/// Maximum legal ARM address > -/// > -#define MAX_ADDRESS 0xFFFFFFFF > - > /// > /// The stack alignment required for ARM > /// > diff --git a/BaseTools/Source/C/Include/Common/UefiBaseTypes.h b/BaseTools/Source/C/Include/Common/UefiBaseTypes.h > index aa1aef3ce638..696ac15e4cd5 100644 > --- a/BaseTools/Source/C/Include/Common/UefiBaseTypes.h > +++ b/BaseTools/Source/C/Include/Common/UefiBaseTypes.h > @@ -170,6 +170,5 @@ typedef union { > > > #define EFI_MAX_BIT MAX_BIT > -#define EFI_MAX_ADDRESS MAX_ADDRESS > > #endif > diff --git a/BaseTools/Source/C/Include/Ia32/ProcessorBind.h b/BaseTools/Source/C/Include/Ia32/ProcessorBind.h > index 4719b53d37fa..96ac691df042 100644 > --- a/BaseTools/Source/C/Include/Ia32/ProcessorBind.h > +++ b/BaseTools/Source/C/Include/Ia32/ProcessorBind.h > @@ -131,11 +131,6 @@ typedef INT32 INTN; > #define MAX_BIT 0x80000000 > #define MAX_2_BITS 0xC0000000 > > -// > -// Maximum legal IA-32 address > -// > -#define MAX_ADDRESS 0xFFFFFFFF > - > // > // Modifier to ensure that all protocol member functions and EFI intrinsics > // use the correct C calling convention. All protocol member functions and > diff --git a/BaseTools/Source/C/Include/X64/ProcessorBind.h b/BaseTools/Source/C/Include/X64/ProcessorBind.h > index c625f8cef4a1..987738508333 100644 > --- a/BaseTools/Source/C/Include/X64/ProcessorBind.h > +++ b/BaseTools/Source/C/Include/X64/ProcessorBind.h > @@ -150,11 +150,6 @@ typedef INT64 INTN; > #define MAX_BIT 0x8000000000000000ULL > #define MAX_2_BITS 0xC000000000000000ULL > > -// > -// Maximum legal Itanium-based address > -// > -#define MAX_ADDRESS 0xFFFFFFFFFFFFFFFFULL > - > // > // Modifier to ensure that all protocol member functions and EFI intrinsics > // use the correct C calling convention. All protocol member functions and > diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c > index 42dfa821624d..5c40fdb5fd49 100644 > --- a/BaseTools/Source/C/Common/CommonLib.c > +++ b/BaseTools/Source/C/Common/CommonLib.c > @@ -1236,7 +1236,6 @@ InternalAllocateCopyPool ( > VOID *Memory; > > ASSERT (Buffer != NULL); > - ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1)); > > Memory = malloc (AllocationSize); > if (Memory != NULL) { > I agree; the malloc() on the next line will cover this. Reviewed-by: Laszlo Ersek Thanks Laszlo