From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (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 F2A8D1A1E4F for ; Tue, 18 Oct 2016 22:29:27 -0700 (PDT) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga105.jf.intel.com with ESMTP; 18 Oct 2016 22:29:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,513,1473145200"; d="scan'208";a="1046770651" Received: from jessica1-mobl.amr.corp.intel.com (HELO localhost) ([10.252.136.91]) by orsmga001.jf.intel.com with ESMTP; 18 Oct 2016 22:29:24 -0700 MIME-Version: 1.0 To: "Zhu, Yonghong" , "edk2-devel@lists.01.org" Message-ID: <147685496347.18024.16872597761474629112@jljusten-ivb> From: Jordan Justen In-Reply-To: Cc: "Gao, Liming" , "Zhu, Yonghong" References: <1476785813-13728-1-git-send-email-yonghong.zhu@intel.com> <147681549396.11463.15081649592826518087@jljusten-ivb> User-Agent: alot/0.3.7 Date: Tue, 18 Oct 2016 22:29:23 -0700 Subject: Re: [Patch 3/3 V2] BaseTools/PatchCheck.py: Update to report error for EFI_D_* X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Oct 2016 05:29:28 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2016-10-18 21:40:16, Zhu, Yonghong wrote: > Hi Jordan, > = > If we change regex to r'''DEBUG \s* \( \s* \(\s*. EFI_D_*''', it cannot > work because we would use mo.group(1) which report IndexError. > If we change regex to r'''DEBUG \s* \( \s* \(\s*. EFI_D_ ([A-Z_]+)''' , > it cannot detect the case "DEBUG ((DEBUG_ERROR | EFI_D_INFO, "xxx", Statu= s));" Sorry, to not be clear. I meant r'''DEBUG \s* \( \s* \( \s* EFI_D_ ([A-Z_]+)''' Here is another option: r''' DEBUG \s* \( \s* \( (?: DEBUG_[A-Z_]+ \s* \| \s*)* EFI_D_ ([A-Z_]+) ''' The down side of .* is that it will first match the entire rest of the line, and then try to match shorter strings. Usually it will end up trying all of the substrings to the end of the string. -Jordan > So how about keep the original regex ? > = > Best Regards, > Zhu Yonghong > = > = > -----Original Message----- > From: Justen, Jordan L = > Sent: Wednesday, October 19, 2016 2:32 AM > To: Zhu, Yonghong ; edk2-devel@lists.01.org > Cc: Gao, Liming > Subject: Re: [Patch 3/3 V2] BaseTools/PatchCheck.py: Update to report err= or for EFI_D_* > = > On 2016-10-18 03:16:53, Yonghong Zhu wrote: > > In EDK2, DEBUG_* is recommended to be used instead of EFI_D_*. For new = > > code, they should use DEBUG_* macro. > > = > > Fixes:https://bugzilla.tianocore.org/show_bug.cgi?id=3D143 > > Cc: Liming Gao > > Cc: Jordan Justen > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Yonghong Zhu > > --- > > BaseTools/Scripts/PatchCheck.py | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > = > > diff --git a/BaseTools/Scripts/PatchCheck.py = > > b/BaseTools/Scripts/PatchCheck.py index 05f8f6e..083438c 100755 > > --- a/BaseTools/Scripts/PatchCheck.py > > +++ b/BaseTools/Scripts/PatchCheck.py > > @@ -338,10 +338,16 @@ class GitDiffCheck: > > lines.append('File: ' + self.hunk_filename) > > lines.append('Line: ' + line) > > = > > self.error(*lines) > > = > > + old_debug_re =3D \ > > + re.compile(r''' > > + DEBUG \s* \( \s* \( .* EFI_D_ ([A-Z_]+) > ^^ > = > I recommend changing .* in the regex to \s*. EFI_D_* is the first paramet= er. The only thing that might appear is a comment, but that could also appe= ar between the parentheses. This would be an unusual place to put a comment. > = > Aside from that, > = > Reviewed-by: Jordan Justen > = > > + ''', > > + re.VERBOSE) > > + > > def check_added_line(self, line): > > eol =3D '' > > for an_eol in self.line_endings: > > if line.endswith(an_eol): > > eol =3D an_eol > > @@ -355,10 +361,16 @@ class GitDiffCheck: > > if ' ' in line: > > self.added_line_error('Tab character used', line) > > if len(stripped) < len(line): > > self.added_line_error('Trailing whitespace found', line) > > = > > + mo =3D self.old_debug_re.search(line) > > + if mo is not None: > > + self.added_line_error('EFI_D_' + mo.group(1) + ' was used,= ' > > + 'but DEBUG_' + mo.group(1) + > > + ' is now recommended', line) > > + > > split_diff_re =3D re.compile(r''' > > (?P > > ^ diff \s+ --git \s+ a/.+ \s+ b= /.+ $ > > ) > > (?P > > -- > > 2.6.1.windows.1 > >=20