[Qemu-devel] [PATCH] target/i386: Correct extra enter and spaces in comment

Tao Xu posted 1 patch 4 years, 7 months ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu failed
Test asan passed
Test checkpatch failed
Test docker-quick@centos7 passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190919020629.26530-1-tao3.xu@intel.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>, Eduardo Habkost <ehabkost@redhat.com>
target/i386/cpu.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[Qemu-devel] [PATCH] target/i386: Correct extra enter and spaces in comment
Posted by Tao Xu 4 years, 7 months ago
There is an extra line in comment of CPUID_8000_0008_EBX_WBNOINVD,
remove the extra enter and spaces.

Signed-off-by: Tao Xu <tao3.xu@intel.com>
---
 target/i386/cpu.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 5f6e3a029a..71b6193390 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -695,8 +695,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
 
 #define CPUID_7_1_EAX_AVX512_BF16 (1U << 5) /* AVX512 BFloat16 Instruction */
 
-#define CPUID_8000_0008_EBX_WBNOINVD  (1U << 9)  /* Write back and
-                                                                             do not invalidate cache */
+#define CPUID_8000_0008_EBX_WBNOINVD (1U << 9) /* Write back and do not invalidate cache */
 #define CPUID_8000_0008_EBX_IBPB    (1U << 12) /* Indirect Branch Prediction Barrier */
 
 #define CPUID_XSAVE_XSAVEOPT   (1U << 0)
-- 
2.20.1


Re: [Qemu-devel] [PATCH] target/i386: Correct extra enter and spaces in comment
Posted by no-reply@patchew.org 4 years, 7 months ago
Patchew URL: https://patchew.org/QEMU/20190919020629.26530-1-tao3.xu@intel.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20190919020629.26530-1-tao3.xu@intel.com
Subject: [Qemu-devel] [PATCH] target/i386: Correct extra enter and spaces in comment
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
3ff8f90 target/i386: Correct extra enter and spaces in comment

=== OUTPUT BEGIN ===
ERROR: line over 90 characters
#23: FILE: target/i386/cpu.h:698:
+#define CPUID_8000_0008_EBX_WBNOINVD (1U << 9) /* Write back and do not invalidate cache */

total: 1 errors, 0 warnings, 9 lines checked

Commit 3ff8f905c3b0 (target/i386: Correct extra enter and spaces in comment) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190919020629.26530-1-tao3.xu@intel.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel] [PATCH] target/i386: Correct extra enter and spaces in comment
Posted by Philippe Mathieu-Daudé 4 years, 7 months ago
Hi Tao,

On 9/19/19 4:06 AM, Tao Xu wrote:
> There is an extra line in comment of CPUID_8000_0008_EBX_WBNOINVD,
> remove the extra enter and spaces.
> 
> Signed-off-by: Tao Xu <tao3.xu@intel.com>
> ---
>  target/i386/cpu.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 5f6e3a029a..71b6193390 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -695,8 +695,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
>  
>  #define CPUID_7_1_EAX_AVX512_BF16 (1U << 5) /* AVX512 BFloat16 Instruction */
>  
> -#define CPUID_8000_0008_EBX_WBNOINVD  (1U << 9)  /* Write back and
> -                                                                             do not invalidate cache */
> +#define CPUID_8000_0008_EBX_WBNOINVD (1U << 9) /* Write back and do not invalidate cache */

The QEMU CODING_STYLE [*] recommend a 80 chars per line limit:

Line width
==========

Lines should be 80 characters; try not to make them longer.

Sometimes it is hard to do, especially when dealing with QEMU subsystems
that use long function or symbol names.  Even in that case, do not make
lines much longer than 80 characters.

Rationale:

* Some people like to tile their 24" screens with a 6x4 matrix of 80x24
  xterms and use vi in all of them.  The best way to punish them is to
  let them keep doing it.
* Code and especially patches is much more readable if limited to a sane
  line length.  Eighty is traditional.
* The four-space indentation makes the most common excuse ("But look
  at all that white space on the left!") moot.
* It is the QEMU coding style.

[*]
https://git.qemu.org/?p=qemu.git;a=blob;f=CODING_STYLE.rst;h=427699e0e425431ea99d4046e40543bdcc22e9c5;hb=HEAD#l82

Can you repost using correct lenght comments?

>  #define CPUID_8000_0008_EBX_IBPB    (1U << 12) /* Indirect Branch Prediction Barrier */
>  
>  #define CPUID_XSAVE_XSAVEOPT   (1U << 0)
> 

Re: [Qemu-devel] [PATCH] target/i386: Correct extra enter and spaces in comment
Posted by Tao Xu 4 years, 7 months ago
On 9/19/2019 5:41 PM, Philippe Mathieu-Daudé wrote:
> Hi Tao,
> 
> On 9/19/19 4:06 AM, Tao Xu wrote:
>> There is an extra line in comment of CPUID_8000_0008_EBX_WBNOINVD,
>> remove the extra enter and spaces.
>>
>> Signed-off-by: Tao Xu <tao3.xu@intel.com>
>> ---
>>   target/i386/cpu.h | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>> index 5f6e3a029a..71b6193390 100644
>> --- a/target/i386/cpu.h
>> +++ b/target/i386/cpu.h
>> @@ -695,8 +695,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
>>   
>>   #define CPUID_7_1_EAX_AVX512_BF16 (1U << 5) /* AVX512 BFloat16 Instruction */
>>   
>> -#define CPUID_8000_0008_EBX_WBNOINVD  (1U << 9)  /* Write back and
>> -                                                                             do not invalidate cache */
>> +#define CPUID_8000_0008_EBX_WBNOINVD (1U << 9) /* Write back and do not invalidate cache */
> 
> The QEMU CODING_STYLE [*] recommend a 80 chars per line limit:
> 
> Line width
> ==========
> 
> Lines should be 80 characters; try not to make them longer.
> 
> Sometimes it is hard to do, especially when dealing with QEMU subsystems
> that use long function or symbol names.  Even in that case, do not make
> lines much longer than 80 characters.
> 
> Rationale:
> 
> * Some people like to tile their 24" screens with a 6x4 matrix of 80x24
>    xterms and use vi in all of them.  The best way to punish them is to
>    let them keep doing it.
> * Code and especially patches is much more readable if limited to a sane
>    line length.  Eighty is traditional.
> * The four-space indentation makes the most common excuse ("But look
>    at all that white space on the left!") moot.
> * It is the QEMU coding style.
> 
> [*]
> https://git.qemu.org/?p=qemu.git;a=blob;f=CODING_STYLE.rst;h=427699e0e425431ea99d4046e40543bdcc22e9c5;hb=HEAD#l82
> 
> Can you repost using correct lenght comments?

Yes, But it is hard to do and in this header file there are lots of 
lines over 80 chars even 90 chars. So how about I correct all the lines 
over 80 like this:

/* Write back and do not invalidate cache */
#define CPUID_8000_0008_EBX_WBNOINVD  (1U << 9)
/* Indirect Branch Prediction Barrier */
#define CPUID_8000_0008_EBX_IBPB    (1U << 12)

> 
>>   #define CPUID_8000_0008_EBX_IBPB    (1U << 12) /* Indirect Branch Prediction Barrier */
>>   
>>   #define CPUID_XSAVE_XSAVEOPT   (1U << 0)
>>