From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web08.598.1612925943353009356 for ; Tue, 09 Feb 2021 18:59:03 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=ouRUsFoD; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [10.124.238.202] (unknown [131.107.174.202]) by linux.microsoft.com (Postfix) with ESMTPSA id CBC8820B6C40; Tue, 9 Feb 2021 18:59:02 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com CBC8820B6C40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1612925942; bh=Wgk4C93wq/VNu7XaQlNpJgWU+R0TVgt8DKAr9Goe1/g=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=ouRUsFoDAtcWApNxXFO0jV5j5pGjfPGYKTmVW9Z2uDzncFhy5SNSH/YA1ozAjuMiY Nf/t7IfneJBilKZnsS2MnGJhc9hQiBeEHiolTcTeAv/OCmsI3wW/4QqaVOza+xtQkS DvBLH7ZBYkRahgl4euAg1aCgH0t1ZBBBdpAGcvi8= Subject: Re: [edk2-devel] [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmCore: Fix compiler warning To: devel@edk2.groups.io, sami.mujawar@arm.com Cc: Ard Biesheuvel , Jiewen Yao , Supreeth Venkatesh , nd References: <20210203035052.402-1-mikuback@linux.microsoft.com> From: "Michael Kubacki" Message-ID: <13a84642-6d7e-cac2-4566-9c36ab9c1e0c@linux.microsoft.com> Date: Tue, 9 Feb 2021 18:59:03 -0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Sami, I'm happy to change the spacing. The code base is very inconsistent with this (a somewhat similar scenario would be space before opening parenthesis) and it often trends toward a space after the cast. The space before opening parenthesis is clearly required. However, I've never managed to find a definitive statement in the EDK II C Coding Standards Specification regarding typecast spacing. For my own future benefit, could you please point me to the definitive statement regarding this rule in the specification? In the specification itself, the following section has no space between the typecast and the variable: https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/v/release%2F2.20/5_source_files/57_c_programming#5-7-2-3-comparison-of-unsigned-integer-types-to-be-greater-than-0-is-permitted The following section does have a space between the typecast and variable: https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/v/release%2F2.20/5_source_files/57_c_programming#5-7-2-4-the-ordering-of-terms-in-predicate-expressions-may-impact-performance-significantly Thanks, Michael On 2/9/2021 2:07 PM, Sami Mujawar wrote: > Hi Michael, > > Please see my response inline marked [SAMI]. > > Other than the minor space change needed to match the coding style, this patch looks good to me. > > With that changed: > Reviewed-by: Sami Mujawar > > Regards, > > Sami Mujawar > > -----Original Message----- > From: mikuback@linux.microsoft.com > Sent: 03 February 2021 03:51 AM > To: devel@edk2.groups.io > Cc: Ard Biesheuvel ; Sami Mujawar ; Jiewen Yao ; Supreeth Venkatesh > Subject: [PATCH v1 1/1] StandaloneMmPkg/StandaloneMmCore: Fix compiler warning > > From: Michael Kubacki > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3204 > > Fixes the following compiler warning in VS2019 by changing defining > the MmramRangeCount variable to be UINTN and type casting prior > to value assignment. > > \edk2\StandaloneMmPkg\Core\StandaloneMmCore.c(570): error C2220: > the following warning is treated as an error > \edk2\StandaloneMmPkg\Core\StandaloneMmCore.c(570): warning C4244: > '=': conversion from 'UINT64' to 'UINT32', possible loss of data > > Cc: Ard Biesheuvel > Cc: Sami Mujawar > Cc: Jiewen Yao > Cc: Supreeth Venkatesh > Signed-off-by: Michael Kubacki > --- > StandaloneMmPkg/Core/StandaloneMmCore.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c b/StandaloneMmPkg/Core/StandaloneMmCore.c > index 8388ec289ca8..d254a68f2fb8 100644 > --- a/StandaloneMmPkg/Core/StandaloneMmCore.c > +++ b/StandaloneMmPkg/Core/StandaloneMmCore.c > @@ -511,7 +511,7 @@ StandaloneMmMain ( > EFI_HOB_GUID_TYPE *MmramRangesHob; > EFI_MMRAM_HOB_DESCRIPTOR_BLOCK *MmramRangesHobData; > EFI_MMRAM_DESCRIPTOR *MmramRanges; > - UINT32 MmramRangeCount; > + UINTN MmramRangeCount; > EFI_HOB_FIRMWARE_VOLUME *BfvHob; > > ProcessLibraryConstructorList (HobStart, &gMmCoreMmst); > @@ -546,7 +546,7 @@ StandaloneMmMain ( > MmramRangesHobData = GET_GUID_HOB_DATA (MmramRangesHob); > ASSERT (MmramRangesHobData != NULL); > MmramRanges = MmramRangesHobData->Descriptor; > - MmramRangeCount = MmramRangesHobData->NumberOfMmReservedRegions; > + MmramRangeCount = (UINTN) MmramRangesHobData->NumberOfMmReservedRegions; > [SAMI] There should be no space between the typecast and the variable, i.e. space after typecast (UINTN) and MmramRangesHobData. > Same at other places in this file. > [/SAMI] > > ASSERT (MmramRanges); > ASSERT (MmramRangeCount); > > @@ -554,7 +554,7 @@ StandaloneMmMain ( > // Copy the MMRAM ranges into MM_CORE_PRIVATE_DATA table just in case any > // code relies on them being present there > // > - gMmCorePrivate->MmramRangeCount = MmramRangeCount; > + gMmCorePrivate->MmramRangeCount = (UINT64) MmramRangeCount; > gMmCorePrivate->MmramRanges = > (EFI_PHYSICAL_ADDRESS)(UINTN)AllocatePool (MmramRangeCount * sizeof (EFI_MMRAM_DESCRIPTOR)); > ASSERT (gMmCorePrivate->MmramRanges != 0); > @@ -567,7 +567,7 @@ StandaloneMmMain ( > DataInHob = GET_GUID_HOB_DATA (GuidHob); > gMmCorePrivate = (MM_CORE_PRIVATE_DATA *)(UINTN)DataInHob->Address; > MmramRanges = (EFI_MMRAM_DESCRIPTOR *)(UINTN)gMmCorePrivate->MmramRanges; > - MmramRangeCount = gMmCorePrivate->MmramRangeCount; > + MmramRangeCount = (UINTN) gMmCorePrivate->MmramRangeCount; > } > > // >