From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (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 52C4721A16EFC for ; Thu, 18 May 2017 13:45:00 -0700 (PDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 May 2017 13:44:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,360,1491289200"; d="scan'208";a="970451865" Received: from jljusten-skl.jf.intel.com (HELO localhost) ([10.54.75.25]) by orsmga003.jf.intel.com with ESMTP; 18 May 2017 13:44:59 -0700 MIME-Version: 1.0 To: Laszlo Ersek , edk2-devel-01 Message-ID: <149514029896.16116.5092031537929111173@jljusten-skl.jf.intel.com> From: Jordan Justen In-Reply-To: Cc: Ard Biesheuvel , Leif Lindholm References: <20170518150427.16435-1-lersek@redhat.com> <20170518150427.16435-2-lersek@redhat.com> <149512810792.14216.11044391728020039744@jljusten-skl> User-Agent: alot/0.5.1 Date: Thu, 18 May 2017 13:44:58 -0700 Subject: Re: [PATCH 1/8] ArmPlatformPkg/NorFlashDxe: correct NumOfLba vararg type in EraseBlocks() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 May 2017 20:45:00 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2017-05-18 12:29:09, Laszlo Ersek wrote: > On 05/18/17 19:21, Jordan Justen wrote: > > On 2017-05-18 08:04:20, Laszlo Ersek wrote: > >> // All blocks must be within range > >> - DEBUG ((DEBUG_BLKIO, "FvbEraseBlocks: Check if: ( StartingLba=3D%= ld + NumOfLba=3D%d - 1 ) > LastBlock=3D%ld.\n", Instance->StartLba + Starti= ngLba, NumOfLba, Instance->Media.LastBlock)); > >> + DEBUG (( > >> + DEBUG_BLKIO, > >> + "FvbEraseBlocks: Check if: ( StartingLba=3D%ld + NumOfLba=3D%Lu= - 1 ) > LastBlock=3D%ld.\n", > > = > > Notably this is still > 80 columns. Maybe? > > = > > "FvbEraseBlocks: Check if: ( StartingLba=3D%ld + NumOfLba=3D%Lu -= 1 ) " > > "> LastBlock=3D%ld.\n", > = > This file ("ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c") has > extremely long lines, the longest one (line 774) has 172 columns. I > broke up the above DEBUG so that it would at least fit in 120 chars per > line (which is the "second level" recommendation in the coding spec). Personally, I don't agree with that secondary 120 char rule. If we ever get the style guide into an 'open source' process, I'd like to suggest removing it. (But, it'll probably get shot down. :\ ) > Also, the format string looked messy enough for me not to want to break > it up. > = > Finally, Leif has expressed a preference earlier for keeping DEBUG > format strings on one line, if memory serves. (I certainly don't follow > that in OvmfPkg and ArmVirtPkg, but ArmPlatformPkg is not my co-turf :) ) Ah. I guess it is fine for a package maintainer to occasionally decide to bend the rules for their package. Hopefully it would not eventually lead to the style migrating into other packages though. -Jordan