From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 F410421959737 for ; Thu, 25 May 2017 08:10:13 -0700 (PDT) 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 528DD627C8; Thu, 25 May 2017 15:10:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 528DD627C8 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 528DD627C8 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-147.phx2.redhat.com [10.3.116.147]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1BC6117DE1; Thu, 25 May 2017 15:10:11 +0000 (UTC) To: Brijesh Singh , edk2-devel@lists.01.org, jordan.l.justen@intel.com Cc: Thomas.Lendacky@amd.com, leo.duran@amd.com References: <1495466592-21641-1-git-send-email-brijesh.singh@amd.com> <1495466592-21641-5-git-send-email-brijesh.singh@amd.com> <868ad318-c652-55c2-4b4c-eeeec2f826c6@redhat.com> From: Laszlo Ersek Message-ID: Date: Thu, 25 May 2017 17:10:10 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: 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.38]); Thu, 25 May 2017 15:10:13 +0000 (UTC) Subject: Re: [PATCH v5 04/14] OvmfPkg/BaseMemcryptSevLib: Add SEV helper library 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, 25 May 2017 15:10:14 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 . 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 . Thanks, Laszlo