[edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395 Add a new api DebugVPrint prototype definition in the DebugLib header file. This api would expose a print routine with VaList parameter. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Zhichao Gao Cc: Michael D Kinney Cc: Liming Gao Cc: Sean Brogan Cc: Michael Turner Cc: Bret Barkelew --- MdePkg/Include/Library/DebugLib.h | 33 - 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h index e6a7a357b2..51d89bbd52 100644 --- a/MdePkg/Include/Library/DebugLib.h +++ b/MdePkg/Include/Library/DebugLib.h @@ -8,7 +8,7 @@ of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is defined, then debug and assert related macros wrapped by it are the NULL implementations. -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License that accompanies this distribution. The full text of the license may be found at @@ -101,6 +101,29 @@ DebugPrint ( ); +/** + Prints a debug message to the debug output device if the specified error level is enabled. + + If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib function + GetDebugPrintErrorLevel (), then print the message specified by Format and the + associated variable argument list to the debug output device. + + If Format is NULL, then ASSERT(). + + @param ErrorLevelThe error level of the debug message. + @param FormatFormat string for the debug message to print. + @param VaListMarker VA_LIST marker for the variable argument list. + +**/ +VOID +EFIAPI +DebugVPrint ( + IN UINTNErrorLevel, + IN CONST CHAR8 *Format, + IN VA_LIST VaListMarker + ); + + /** Prints an assert message containing a filename, line number, and description. This may be followed by a breakpoint or a dead loop. @@ -221,6 +244,7 @@ DebugClearMemoryEnabled ( VOID ); + /** Returns TRUE if any one of the bit is set both in ErrorLevel and PcdFixedDebugPrintErrorLevel. @@ -236,6 +260,7 @@ DebugPrintLevelEnabled ( IN CONST UINTNErrorLevel ); + /** Internal worker macro that calls DebugAssert(). @@ -273,6 +298,7 @@ DebugPrintLevelEnabled ( #define _DEBUG(Expression) DebugPrint Expression #endif + /** Macro that calls DebugAssert() if an expression evaluates to FALSE. @@ -299,6 +325,7 @@ DebugPrintLevelEnabled ( #define ASSERT(Expression) #endif + /** Macro that calls DebugPrint(). @@ -322,6 +349,7 @@ DebugPrintLevelEnabled ( #define DEBUG(Expression) #endif + /** Macro that calls DebugAssert() if an EFI_STATUS evaluates to an error code. @@ -348,6 +376,7 @@ DebugPrintLevelEnabled ( #define ASSERT_EFI_ERROR(StatusParameter) #endif + /** Macro that calls DebugAssert() if a RETURN_STATUS evaluates to an error code. @@ -375,6 +404,7 @@ DebugPrintLevelEnabled ( #define ASSERT_RETURN_ERROR(StatusParameter) #endif + /** Macro that calls DebugAssert() if a protocol is already installed in the handle database. @@ -418,6 +448,7 @@ DebugPrintLevelEnabled ( #define ASSERT_PROTOCOL_ALREADY_INSTALLED(Handle, Guid) #endif + /** Macro that marks the beginning of debug source code. -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
On 2019-03-14 22:17:33, Zhichao Gao wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395 > > Add a new api DebugVPrint prototype definition in the > DebugLib header file. This api would expose a print > routine with VaList parameter. These lines seem to be fairly short, with the longest be 54 chars. I guess not a problem, but by the recommendation says they could be up to 75 in length. https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format > > diff --git a/MdePkg/Include/Library/DebugLib.h > b/MdePkg/Include/Library/DebugLib.h > index e6a7a357b2..51d89bbd52 100644 > --- a/MdePkg/Include/Library/DebugLib.h > +++ b/MdePkg/Include/Library/DebugLib.h > @@ -8,7 +8,7 @@ >of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG > is >defined, then debug and assert related macros wrapped by it are the NULL > implementations. > > -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. > +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. > This program and the accompanying materials are licensed and made available > under > the terms and conditions of the BSD License that accompanies this > distribution. > The full text of the license may be found at > @@ -101,6 +101,29 @@ DebugPrint ( >); > > +/** > + Prints a debug message to the debug output device if the specified error > level is enabled. According to the style guide: "Preferably, limit line lengths to 80 columns or less." https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C https://github.com/tianocore-docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf But, this line uses 92 columns. I think there are similar cases in other patches. > + > + If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib function > + GetDebugPrintErrorLevel (), then print the message specified by Format and > the > + associated variable argument list to the debug output device. > + > + If Format is NULL, then ASSERT(). > + > + @param ErrorLevelThe error level of the debug message. > + @param FormatFormat string for the debug message to print. > + @param VaListMarker VA_LIST marker for the variable argument list. > + > +**/ > +VOID > +EFIAPI > +DebugVPrint ( > + IN UINTNErrorLevel, > + IN CONST CHAR8 *Format, > + IN VA_LIST VaListMarker > + ); > + > + > /** >Prints an assert message containing a filename, line number, and > description. >This may be followed by a breakpoint or a dead loop. > @@ -221,6 +244,7 @@ DebugClearMemoryEnabled ( >VOID >); > > + What's going on here? It seems like maybe extra lines are added that have nothing to do with this patch. -Jordan > /** >Returns TRUE if any one of the bit is set both in ErrorLevel and > PcdFixedDebugPrintErrorLevel. > > @@ -236,6 +260,7 @@ DebugPrintLevelEnabled ( >IN CONST UINTNErrorLevel >); > > + > /** >Internal worker macro that calls DebugAssert(). > > @@ -273,6 +298,7 @@ DebugPrintLevelEnabled ( > #define _DEBUG(Expression) DebugPrint Expression > #endif > > + > /** >Macro that calls DebugAssert() if an expression evaluates to FALSE. > > @@ -299,6 +325,7 @@ DebugPrintLevelEnabled ( >#define ASSERT(Expression) > #endif > > + > /** >Macro that calls DebugPrint(). > > @@ -322,6 +349,7 @@ DebugPrintLevelEnabled ( >#define DEBUG(Expression) > #endif > > + > /** >Macro that calls DebugAssert() if an EFI_STATUS evaluates to an error code. > > @@ -348,6 +376,7 @@ DebugPrintLevelEnabled ( >#define ASSERT_EFI_ERROR(StatusParameter) > #endif > > + > /** >Macro that calls DebugAssert() if a RETURN_STATUS evaluates to an error > code. > > @@ -375,6 +404,7 @@ DebugPrintLevelEnabled ( >#define ASSERT_RETURN_ERROR(StatusParameter) > #endif > > + > /** >Macro that calls DebugAssert() if a protocol is already installed in the >handle database. > @@ -418,6 +448,7 @@ DebugPrintLevelEnabled ( >#define ASSERT_PROTOCOL_ALREADY_INSTALLED(Handle, Guid) > #endif > > + > /** >Macro that marks the beginning of debug source code. > > -- > 2.16.2.windows.1 > > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
> -Original Message- > From: Justen, Jordan L > Sent: Friday, March 15, 2019 2:16 PM > To: Gao, Zhichao ; edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Bret Barkelew > ; Michael Turner > ; Gao, Liming > Subject: Re: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api > DebugVPrint for DebugLib > > On 2019-03-14 22:17:33, Zhichao Gao wrote: > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395 > > > > Add a new api DebugVPrint prototype definition in the DebugLib header > > file. This api would expose a print routine with VaList parameter. > > These lines seem to be fairly short, with the longest be 54 chars. I guess > not a > problem, but by the recommendation says they could be up to 75 in length. > > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message- > Format > It is hard for someone to control the characters in a line. Making the text massage readable base on the rules make more sense. It is easier to control the chars number within 75. Line length less than 76 characters is legal. > > > > diff --git a/MdePkg/Include/Library/DebugLib.h > > b/MdePkg/Include/Library/DebugLib.h > > index e6a7a357b2..51d89bbd52 100644 > > --- a/MdePkg/Include/Library/DebugLib.h > > +++ b/MdePkg/Include/Library/DebugLib.h > > @@ -8,7 +8,7 @@ > >of size reduction when compiler optimization is disabled. If > MDEPKG_NDEBUG is > >defined, then debug and assert related macros wrapped by it are the > NULL implementations. > > > > -Copyright (c) 2006 - 2018, Intel Corporation. All rights > > reserved. > > +Copyright (c) 2006 - 2019, Intel Corporation. All rights > > +reserved. > > This program and the accompanying materials are licensed and made > > available under the terms and conditions of the BSD License that > accompanies this distribution. > > The full text of the license may be found at @@ -101,6 +101,29 @@ > > DebugPrint ( > >); > > > > +/** > > + Prints a debug message to the debug output device if the specified error > level is enabled. > > According to the style guide: > > "Preferably, limit line lengths to 80 columns or less." > > https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C > > https://github.com/tianocore- > docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf > > But, this line uses 92 columns. > > I think there are similar cases in other patches. Right. This sentence I copied from the comment of DebugPrint function which might be designed for a long time. The CCS might not be designed as such style at that time. For this, it's better to change both of the two sentences. By the way, the whole edk2 repo may contain a lot of lines violated this rule. It takes effort to fix them all. > > > + > > + If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib > > + function GetDebugPrintErrorLevel (), then print the message > > + specified by Format and the associated variable argument list to the > debug output device. > > + > > + If Format is NULL, then ASSERT(). > > + > > + @param ErrorLevelThe error level of the debug message. > > + @param FormatFormat string for the debug message to print. > > + @param VaListMarker VA_LIST marker for the variable argument list. > > + > > +**/ > > +VOID > > +EFIAPI > > +DebugVPrint ( > > + IN UINTNErrorLevel, > > + IN CONST CHAR8 *Format, > > + IN VA_LIST VaListMarker > > + ); > > + > > + > > /** > >Prints an assert message containing a filename, line number, and > description. > >This may be followed by a breakpoint or a dead loop. > > @@ -221,6 +244,7 @@ DebugClearMemoryEnabled ( > >VOID > >); > > > > + > > What's going on here? It seems like maybe extra lines are added that have > nothing to do with this patch. > > -Jordan > For this library DebugLib, seems it prefers to put two blank lines between functions. I just want to make it tidy. Maybe it is an inappropriate behavior. But there are no rules for how much blank lines should be putted between functions. For my own, one single blank line is enough. Both adding blank lines and deleting blank lines seem inappropriate. Or just stay the original style? Thanks, Zhichao > > /** > >Returns TRUE if any one of the bit is set both in ErrorLevel and > PcdFixedDebugPrintErrorLevel. > > > > @@ -236,6 +260,7 @@ DebugPrintLevelEnabled ( > >IN CONST UINTNErrorLevel > >); > > > > + > > /** > >Inter
Re: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
On 2019-03-17 17:14:40, Gao, Zhichao wrote: > > -Original Message- > > From: Justen, Jordan L > > Sent: Friday, March 15, 2019 2:16 PM > > > > On 2019-03-14 22:17:33, Zhichao Gao wrote: > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395 > > > > > > Add a new api DebugVPrint prototype definition in the > > > DebugLib header file. This api would expose a print > > > routine with VaList parameter. > > > > These lines seem to be fairly short, with the longest be 54 chars. I guess > > not a > > problem, but by the recommendation says they could be up to 75 in length. > > > > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message- > > Format > > > > It is hard for someone to control the characters in a line. I guess that depends on your editor. :) > Making the text massage readable base on the rules make more sense. > It is easier to control the chars number within 75. Line length less > than 76 characters is legal. Yes, I mentioned it is legal, but it is noticably short. I don't think it is so short that it is really a problem, so feel free to leave it if you prefer. > > > > According to the style guide: > > > > "Preferably, limit line lengths to 80 columns or less." > > > > https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C > > > > https://github.com/tianocore- > > docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf > > > > But, this line uses 92 columns. > > > > I think there are similar cases in other patches. > > Right. This sentence I copied from the comment of DebugPrint > function which might be designed for a long time. The CCS might not > be designed as such style at that time. For this, it's better to > change both of the two sentences. > By the way, the whole edk2 repo may contain a lot of lines violated > this rule. It takes effort to fix them all. I agree there are many violations of this in edk2. Usually, it's not too important to go around fixing the issues that already happened, but if you are changing or adding some code, then you can try to make sure the new lines you add are good. -Jordan ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
Jordan: Coding style document CCS_2_1_Draft.pdf 5.1 General Rules, 5.1.1 Lines shall be 120 columns, or less. Preferably, limit line lengths to 80 columns or less. So, I don't think the line length bigger than 80 and less than 120 violates coding style documents. We should update wiki https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C to match the documentation. Limit line length to 120 characters instead of 80 characters. Thanks Liming > -Original Message- > From: Justen, Jordan L > Sent: Monday, March 18, 2019 2:21 PM > To: Gao, Zhichao ; edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Bret Barkelew > ; Michael Turner > ; Gao, Liming > Subject: RE: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api > DebugVPrint for DebugLib > > On 2019-03-17 17:14:40, Gao, Zhichao wrote: > > > -Original Message- > > > From: Justen, Jordan L > > > Sent: Friday, March 15, 2019 2:16 PM > > > > > > On 2019-03-14 22:17:33, Zhichao Gao wrote: > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395 > > > > > > > > Add a new api DebugVPrint prototype definition in the > > > > DebugLib header file. This api would expose a print > > > > routine with VaList parameter. > > > > > > These lines seem to be fairly short, with the longest be 54 chars. I > > > guess not a > > > problem, but by the recommendation says they could be up to 75 in length. > > > > > > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message- > > > Format > > > > > > > It is hard for someone to control the characters in a line. > > I guess that depends on your editor. :) > > > Making the text massage readable base on the rules make more sense. > > It is easier to control the chars number within 75. Line length less > > than 76 characters is legal. > > Yes, I mentioned it is legal, but it is noticably short. I don't think > it is so short that it is really a problem, so feel free to leave it > if you prefer. > > > > > > > According to the style guide: > > > > > > "Preferably, limit line lengths to 80 columns or less." > > > > > > https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C > > > > > > https://github.com/tianocore- > > > docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf > > > > > > But, this line uses 92 columns. > > > > > > I think there are similar cases in other patches. > > > > Right. This sentence I copied from the comment of DebugPrint > > function which might be designed for a long time. The CCS might not > > be designed as such style at that time. For this, it's better to > > change both of the two sentences. > > By the way, the whole edk2 repo may contain a lot of lines violated > > this rule. It takes effort to fix them all. > > I agree there are many violations of this in edk2. > > Usually, it's not too important to go around fixing the issues that > already happened, but if you are changing or adding some code, then > you can try to make sure the new lines you add are good. > > -Jordan ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
On 2019-03-18 08:09:36, Gao, Liming wrote: > Jordan: > Coding style document CCS_2_1_Draft.pdf 5.1 General Rules, 5.1.1 > Lines shall be 120 columns, or less. Preferably, limit line > lengths to 80 columns or less. So, I don't think the line length > bigger than 80 and less than 120 violates coding style documents. I think this is clarified as an "exception" base: "When this doesn’t leave sufficient space for a good postfix style comment, extend the line to a total of 120 columns." That doesn't apply to this case. I have never seen a case where I thought this exception was useful in EDK II. I think the exception should be removed, because if a true exceptional case ever did arise, it would be so obvious, no one would need to question it. The only case I've seen (not in EDK II) where going beyond 80 columns seemed to make some sense was a table processed by a build tool. In that case the table has so many columns that line went beyond 80 columns. There a good code-readability reasons for this limit. For one, we can be almost certain that everyone will have 80 columns visible in their editor. But, just as importantly, as the line gets too long, it becomes more challenging to track back to the start of the next line. (Think of newspaper columns, and imagine if they ran all the way across the page.) I'm not sure 80 columns is the "perfect" number, but it does seem that most agree it is probably close. > We should update wiki > https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C > to match the documentation. Limit line length to 120 characters > instead of 80 characters. Since this is an "exception", and not normally helpful, I don't think we should add it do the wiki. It will only add confusion and lead to more code that doesn't follow the preferred limit. -Jordan > > > -Original Message- > > From: Justen, Jordan L > > Sent: Monday, March 18, 2019 2:21 PM > > To: Gao, Zhichao ; edk2-devel@lists.01.org > > Cc: Kinney, Michael D ; Bret Barkelew > > ; Michael Turner > > ; Gao, Liming > > Subject: RE: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api > > DebugVPrint for DebugLib > > > > On 2019-03-17 17:14:40, Gao, Zhichao wrote: > > > > -Original Message- > > > > From: Justen, Jordan L > > > > Sent: Friday, March 15, 2019 2:16 PM > > > > > > > > On 2019-03-14 22:17:33, Zhichao Gao wrote: > > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395 > > > > > > > > > > Add a new api DebugVPrint prototype definition in the > > > > > DebugLib header file. This api would expose a print > > > > > routine with VaList parameter. > > > > > > > > These lines seem to be fairly short, with the longest be 54 chars. I > > > > guess not a > > > > problem, but by the recommendation says they could be up to 75 in > > > > length. > > > > > > > > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message- > > > > Format > > > > > > > > > > It is hard for someone to control the characters in a line. > > > > I guess that depends on your editor. :) > > > > > Making the text massage readable base on the rules make more sense. > > > It is easier to control the chars number within 75. Line length less > > > than 76 characters is legal. > > > > Yes, I mentioned it is legal, but it is noticably short. I don't think > > it is so short that it is really a problem, so feel free to leave it > > if you prefer. > > > > > > > > > > According to the style guide: > > > > > > > > "Preferably, limit line lengths to 80 columns or less." > > > > > > > > https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C > > > > > > > > https://github.com/tianocore- > > > > docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf > > > > > > > > But, this line uses 92 columns. > > > > > > > > I think there are similar cases in other patches. > > > > > > Right. This sentence I copied from the comment of DebugPrint > > > function which might be designed for a long time. The CCS might not > > > be designed as such style at that time. For this, it's better to > > > change both of the two sentences. > > > By the way, the whole edk2 repo may contain a lot of lines violated > > > this rule. It takes effort to fix them all. > > > > I agree there are many violations of this in edk2. > > > > Usually, it's not too important to go around fixing the issues that > > already happened, but if you are changing or adding some code, then > > you can try to make sure the new lines you add are good. > > > > -Jordan ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
Jordan: I suggest to add the line length discussion in edk2 community meeting. Which length is better, 80 or 120? If we expect every patch follows this rule, do we need update PatchCheck to add the checker, and notify the developer know this requirement? For now, I suggest that the developer bases on current process, pass PatchCheck and code review. Thanks Liming >-Original Message- >From: Justen, Jordan L >Sent: Tuesday, March 19, 2019 2:34 AM >To: Gao, Liming ; Gao, Zhichao >; edk2-devel@lists.01.org >Cc: Kinney, Michael D ; Bret Barkelew >; Michael Turner >; Gao, Liming >Subject: RE: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api >DebugVPrint for DebugLib > >On 2019-03-18 08:09:36, Gao, Liming wrote: >> Jordan: >> Coding style document CCS_2_1_Draft.pdf 5.1 General Rules, 5.1.1 >> Lines shall be 120 columns, or less. Preferably, limit line >> lengths to 80 columns or less. So, I don't think the line length >> bigger than 80 and less than 120 violates coding style documents. > >I think this is clarified as an "exception" base: "When this doesn’t >leave sufficient space for a good postfix style comment, extend the >line to a total of 120 columns." > >That doesn't apply to this case. > >I have never seen a case where I thought this exception was useful in >EDK II. I think the exception should be removed, because if a true >exceptional case ever did arise, it would be so obvious, no one would >need to question it. > >The only case I've seen (not in EDK II) where going beyond 80 columns >seemed to make some sense was a table processed by a build tool. In >that case the table has so many columns that line went beyond 80 >columns. > >There a good code-readability reasons for this limit. For one, we can >be almost certain that everyone will have 80 columns visible in their >editor. But, just as importantly, as the line gets too long, it >becomes more challenging to track back to the start of the next line. >(Think of newspaper columns, and imagine if they ran all the way >across the page.) I'm not sure 80 columns is the "perfect" number, but >it does seem that most agree it is probably close. > >> We should update wiki >> https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C >> to match the documentation. Limit line length to 120 characters >> instead of 80 characters. > >Since this is an "exception", and not normally helpful, I don't think >we should add it do the wiki. It will only add confusion and lead to >more code that doesn't follow the preferred limit. > >-Jordan > >> >> > -----Original Message----- >> > From: Justen, Jordan L >> > Sent: Monday, March 18, 2019 2:21 PM >> > To: Gao, Zhichao ; edk2-devel@lists.01.org >> > Cc: Kinney, Michael D ; Bret Barkelew >; Michael Turner >> > ; Gao, Liming >> > Subject: RE: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api >DebugVPrint for DebugLib >> > >> > On 2019-03-17 17:14:40, Gao, Zhichao wrote: >> > > > -Original Message- >> > > > From: Justen, Jordan L >> > > > Sent: Friday, March 15, 2019 2:16 PM >> > > > >> > > > On 2019-03-14 22:17:33, Zhichao Gao wrote: >> > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395 >> > > > > >> > > > > Add a new api DebugVPrint prototype definition in the >> > > > > DebugLib header file. This api would expose a print >> > > > > routine with VaList parameter. >> > > > >> > > > These lines seem to be fairly short, with the longest be 54 chars. I >guess not a >> > > > problem, but by the recommendation says they could be up to 75 in >length. >> > > > >> > > > https://github.com/tianocore/tianocore.github.io/wiki/Commit- >Message- >> > > > Format >> > > > >> > > >> > > It is hard for someone to control the characters in a line. >> > >> > I guess that depends on your editor. :) >> > >> > > Making the text massage readable base on the rules make more sense. >> > > It is easier to control the chars number within 75. Line length less >> > > than 76 characters is legal. >> > >> > Yes, I mentioned it is legal, but it is noticably short. I don't think >> > it is so short that it is really a problem, so feel free to leave it >> > if you prefer. >> > >> > > > >> > > > Acc
Re: [edk2] [PATCH V2 01/17] MdePkg/DebugLib.h: Add a new api DebugVPrint for DebugLib
On 3/19/2019 7:45 AM, Gao, Liming wrote: Jordan: I suggest to add the line length discussion in edk2 community meeting. Which length is better, 80 or 120? If we expect every patch follows this rule, do we need update PatchCheck to add the checker, and notify the developer know this requirement? I've added this discussion to the list for next month's community meeting: https://www.tianocore.org/community-meeting/ Cheers, Stephano ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
