Re: [PATCH 1/8] ArmPlatformPkg/NorFlashDxe: correct NumOfLba vararg type in EraseBlocks()

[Jordan Justen <jordan.l.justen@intel.com>]

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=%ld + NumOfLba=%d - 1 ) > LastBlock=%ld.\n", Instance->StartLba + StartingLba, NumOfLba, Instance->Media.LastBlock));
> >> +    DEBUG ((
> >> +      DEBUG_BLKIO,
> >> +      "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) > LastBlock=%ld.\n",
> > 
> > Notably this is still > 80 columns. Maybe?
> > 
> >       "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) "
> >       "> LastBlock=%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