Re: [PATCH v5 04/14] OvmfPkg/BaseMemcryptSevLib: Add SEV helper library

[Laszlo Ersek <lersek@redhat.com>]

On 05/25/17 00:12, Brijesh Singh wrote:
>
> Hi Laszlo,
>
> On 05/24/2017 08:06 AM, Laszlo Ersek wrote:
>>
>> (2) please check the lines where you added (as I asked, thanks)
>> gEfiCallerBaseName and __FUNCTION__. On most lines, the indentation
>> is incorrect, relative to "DEBUG ((".
>
> Just so I get it right this time, can you please confirm that below
> indentation is correct:
>
> DEBUG ((DEBUG_VERBOSE, "%a:%a Set C-bit Cr3 %Lx Base %Lx Length %Lx
> flush %d\n",
>   gEfiCallerBaseName, __FUNCTION__, Cr3BaseAddress, PhysicalAddress,
> Length, Flush));
>
> I was trying to look into other files and some have different style,
> and checkpatch didn't complain the formatting error hence I thought
> what I had was correct.

The canonical way to write this DEBUG invocation is:

  DEBUG ((
    DEBUG_VERBOSE,
    "%a:%a Set C-bit Cr3 %Lx Base %Lx Length %Lx flush %d\n",
    gEfiCallerBaseName,
    __FUNCTION__,
    Cr3BaseAddress,
    PhysicalAddress,
    Length,
    Flush
    ));

(Do not miss the indentation of the closing paren(s)!)

Please refer to <https://bugzilla.tianocore.org/show_bug.cgi?id=425>.

If it all fits on a single line, not exceeding 80 characters, then you
can keep it on a single line.

Otherwise, if you don't fit on a single line, then you have to break
every argument to a separate line. If your format string (or any other
argument) doesn't fit on a line in itself, then you have to break it up
too.

Earlier I'd been using a "meet in the middle" style, where I wouldn't
exceed 80 characters per line, and would indent the continuations by 2
additional spaces, but still wouldn't break each argument to a new line.
Example:

  DEBUG ((DEBUG_VERBOSE,
    "%a:%a Set C-bit Cr3 %Lx Base %Lx Length %Lx flush %d\n",
    gEfiCallerBaseName, __FUNCTION__, Cr3BaseAddress, PhysicalAddress, Length,
    Flush));

In my opinion, this would be the best compromise, since (a) it keeps
lines under 80 chars width, (b) conforms to the indentation requirement,
(c) doesn't waste vertical space like the official layout above.

However, this style had not been approved, and I abandoned it in favor
of the canonical style, when I filed
<https://bugzilla.tianocore.org/show_bug.cgi?id=425>.

Thanks,
Laszlo