From mboxrd@z Thu Jan  1 00:00:00 1970
Authentication-Results: mx.groups.io;
 dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com)
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
 by groups.io with SMTP; Wed, 17 Jul 2019 07:42:16 -0700
Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])
	(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
	(No client certificate requested)
	by mx1.redhat.com (Postfix) with ESMTPS id 61B73308404B;
	Wed, 17 Jul 2019 14:42:16 +0000 (UTC)
Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-5.ams2.redhat.com [10.36.117.5])
	by smtp.corp.redhat.com (Postfix) with ESMTP id B835D60920;
	Wed, 17 Jul 2019 14:42:12 +0000 (UTC)
Subject: Re: [edk2-devel] [PATCH] MdePkg/BaseLib: Base64Decode(): don't declare variables in nested blocks
From: "Laszlo Ersek" <lersek@redhat.com>
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Liming Gao <liming.gao@intel.com>,
 Michael D Kinney <michael.d.kinney@intel.com>,
 =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= <philmd@redhat.com>,
 Zhichao Gao <zhichao.gao@intel.com>, Rebecca Cran <rebecca@bsdio.com>
Reply-To: devel@edk2.groups.io, lersek@redhat.com
References: <20190716225926.19732-1-lersek@redhat.com>
Message-ID: <346b3244-ba74-ed11-c91f-7d2fa7c562b4@redhat.com>
Date: Wed, 17 Jul 2019 16:42:11 +0200
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: <20190716225926.19732-1-lersek@redhat.com>
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.40]); Wed, 17 Jul 2019 14:42:16 +0000 (UTC)
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable

On 07/17/19 00:59, Laszlo Ersek wrote:
> The EDK II C Coding Standards Specification (v2.20) strongly discourage=
s
> variable declarations in nested block scope:
>=20
> 5       Source Files
> 5.4     Code File Structure
> 5.4.1   Scoping Rules
> 5.4.1.1 Scope
>=20
>> Block (local) Scope
>>
>> [...]
>>
>> Data declarations may follow the opening brace of a compound statement=
,
>> regardless of nesting depth, and before any code generating statements
>> have been entered. Other than at the outermost block of a function bod=
y,
>> this type of declaration is strongly discouraged.
>=20
> Hoist such variable declarations in Base64Decode() to the outermost
> function scope.
>=20
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Philippe Mathieu-Daud=C3=A9 <philmd@redhat.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Fixes: 35e242b698cdc6205e99a6d6a188bf27fecf9fb4
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1980
> Reported-by: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>=20
> Notes:
>     Repo:   https://github.com/lersek/edk2.git
>     Branch: b64_decode_no_nested_bz1980
>=20
>  MdePkg/Library/BaseLib/String.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

Thank you all for the reviews / testing; patch pushed as commit
cce01f538fb4.

Thanks!
Laszlo

> diff --git a/MdePkg/Library/BaseLib/String.c b/MdePkg/Library/BaseLib/S=
tring.c
> index 6198ccbc9672..45198373f25c 100644
> --- a/MdePkg/Library/BaseLib/String.c
> +++ b/MdePkg/Library/BaseLib/String.c
> @@ -1978,6 +1978,9 @@ Base64Decode (
>    UINT32  Accumulator;
>    UINTN   OriginalDestinationSize;
>    UINTN   SourceIndex;
> +  CHAR8   SourceChar;
> +  UINT32  Base64Value;
> +  UINT8   DestinationOctet;
> =20
>    if (DestinationSize =3D=3D NULL) {
>      return RETURN_INVALID_PARAMETER;
> @@ -2054,10 +2057,6 @@ Base64Decode (
>    // Decoding loop.
>    //
>    for (SourceIndex =3D 0; SourceIndex < SourceSize; SourceIndex++) {
> -    CHAR8  SourceChar;
> -    UINT32 Base64Value;
> -    UINT8  DestinationOctet;
> -
>      SourceChar =3D Source[SourceIndex];
> =20
>      //
>=20