[Qemu-devel] [PATCH 0/2] CODING_STYLE: trivial update

[Qemu-devel] [PATCH 0/2] CODING_STYLE: trivial update

[Wei Yang]

The first one is suggested by Igor Mammedov to provide rule for multiline
code.

The second is a trivial fix to make example code all indented with 4 spaces.

Wei Yang (2):
  CODING_STYLE: specify the indent rule for multiline code
  CODING_STYLE: indent example code as all others

 CODING_STYLE | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

-- 
2.19.1

[Qemu-devel] [PATCH 1/2] CODING_STYLE: specify the indent rule for multiline code

[Wei Yang]

We didn't specify the indent rule for multiline code here, which may
misleading users. And in current code, the code use different rules.

Add this rule in CODING_STYLE to make sure this is clear to every one.

Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Suggested-by: Igor Mammedov <imammedo@redhat.com>
---
 CODING_STYLE | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/CODING_STYLE b/CODING_STYLE
index ec075dedc4..73f66ca185 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -29,6 +29,32 @@ Spaces of course are superior to tabs because:
 
 Do not leave whitespace dangling off the ends of lines.
 
+1.1 Multiline Indent
+
+There are several places where indent is necessary:
+
+ - struct definition
+ - if/else
+ - while/for
+ - function definition & call
+
+All the above cases apply the same rule: indent with four spaces.
+
+While the last three case may face another situation: code should spread into
+several lines. In this case the rule is align the new line with first
+parentheses.
+
+For example:
+
+    if (a == 1 &&
+        b == 2)
+
+    while (a == 1 &&
+           b == 2)
+
+    do_something(arg1, arg2
+                 arg3)
+
 2. Line width
 
 Lines should be 80 characters; try not to make them longer.
-- 
2.19.1

[Qemu-devel] [PATCH 2/2] CODING_STYLE: indent example code as all others

[Wei Yang]

All the example code are indented with four spaces except this one.

Fix this by adding four spaces here.

Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
 CODING_STYLE | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/CODING_STYLE b/CODING_STYLE
index 73f66ca185..27581d80c1 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -134,10 +134,10 @@ block to a separate function altogether.
 When comparing a variable for (in)equality with a constant, list the
 constant on the right, as in:
 
-if (a == 1) {
-    /* Reads like: "If a equals 1" */
-    do_something();
-}
+    if (a == 1) {
+        /* Reads like: "If a equals 1" */
+        do_something();
+    }
 
 Rationale: Yoda conditions (as in 'if (1 == a)') are awkward to read.
 Besides, good compilers already warn users when '==' is mis-typed as '=',
-- 
2.19.1

Re: [Qemu-devel] [PATCH 1/2] CODING_STYLE: specify the indent rule for multiline code

[Philippe Mathieu-Daudé]

Hi,

On 2/19/19 2:31 AM, Wei Yang wrote:
> We didn't specify the indent rule for multiline code here, which may
> misleading users. And in current code, the code use different rules.
> 
> Add this rule in CODING_STYLE to make sure this is clear to every one.
> 
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  CODING_STYLE | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/CODING_STYLE b/CODING_STYLE
> index ec075dedc4..73f66ca185 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -29,6 +29,32 @@ Spaces of course are superior to tabs because:
>  
>  Do not leave whitespace dangling off the ends of lines.
>  
> +1.1 Multiline Indent
> +
> +There are several places where indent is necessary:
> +
> + - struct definition
> + - if/else
> + - while/for
> + - function definition & call
> +
> +All the above cases apply the same rule: indent with four spaces.
> +
> +While the last three case may face another situation: code should spread into
> +several lines. In this case the rule is align the new line with first
> +parentheses.
> +
> +For example:
> +
> +    if (a == 1 &&
> +        b == 2)
> +
> +    while (a == 1 &&
> +           b == 2)
> +
> +    do_something(arg1, arg2
> +                 arg3)

Thanks for clearing this.

What is still unclear is what to do when a function name is over 60
characters (you follow a library/API and can not shorten it), for example:

static void ccid_card_vscard_handle_message(PassthruState *card,
    const VSCMsgHeader *scr_msg_header);

What is the project guideline in this case?
(Cc'ed Peter).

> +
>  2. Line width
>  
>  Lines should be 80 characters; try not to make them longer.
> 

Re: [Qemu-devel] [PATCH 2/2] CODING_STYLE: indent example code as all others

[Philippe Mathieu-Daudé]

On 2/19/19 2:31 AM, Wei Yang wrote:
> All the example code are indented with four spaces except this one.
> 
> Fix this by adding four spaces here.
> 
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
>  CODING_STYLE | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/CODING_STYLE b/CODING_STYLE
> index 73f66ca185..27581d80c1 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -134,10 +134,10 @@ block to a separate function altogether.
>  When comparing a variable for (in)equality with a constant, list the
>  constant on the right, as in:
>  
> -if (a == 1) {
> -    /* Reads like: "If a equals 1" */
> -    do_something();
> -}
> +    if (a == 1) {
> +        /* Reads like: "If a equals 1" */

I guess you found a bug in the documentation :)

Since 8c06fbdf36bf4d the style asked is:

    We now require Linux-kernel-style multiline comments:
        /*
         * line one
         * line two
         */

> +        do_something();
> +    }
>  
>  Rationale: Yoda conditions (as in 'if (1 == a)') are awkward to read.
>  Besides, good compilers already warn users when '==' is mis-typed as '=',
> 

Re: [Qemu-devel] [PATCH 1/2] CODING_STYLE: specify the indent rule for multiline code

[Eric Blake]

On 2/18/19 7:31 PM, Wei Yang wrote:
> We didn't specify the indent rule for multiline code here, which may
> misleading users. And in current code, the code use different rules.
> 
> Add this rule in CODING_STYLE to make sure this is clear to every one.
> 
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  CODING_STYLE | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/CODING_STYLE b/CODING_STYLE
> index ec075dedc4..73f66ca185 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -29,6 +29,32 @@ Spaces of course are superior to tabs because:
>  
>  Do not leave whitespace dangling off the ends of lines.
>  
> +1.1 Multiline Indent
> +
> +There are several places where indent is necessary:
> +
> + - struct definition
> + - if/else
> + - while/for
> + - function definition & call
> +
> +All the above cases apply the same rule: indent with four spaces.

Is this redundant with the earlier statement that "QEMU indents are four
spaces."?

> +
> +While the last three case may face another situation: code should spread into
> +several lines. In this case the rule is align the new line with first
> +parentheses.

Grammar is awkward - the leading 'while' makes it sound like that you
have a dependent clause, but then never provide the independent clause.
Maybe a completely different wording is better:

When breaking up a long line to fit within line widths, align the
secondary lines just after the opening parenthesis of the first.

> +
> +For example:
> +
> +    if (a == 1 &&
> +        b == 2)
> +

Maybe:

if (a == 1 &&
    b == 2) {

to match our later recommendations on always using {}.

> +    while (a == 1 &&
> +           b == 2)
> +

Similar.

> +    do_something(arg1, arg2
> +                 arg3)
> +

and here, I'd include the ';' to make it a valid statement.

>  2. Line width
>  
>  Lines should be 80 characters; try not to make them longer.
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 1/2] CODING_STYLE: specify the indent rule for multiline code

[Eric Blake]

On 2/19/19 11:34 AM, Philippe Mathieu-Daudé wrote:

> 
> What is still unclear is what to do when a function name is over 60
> characters (you follow a library/API and can not shorten it), for example:
> 
> static void ccid_card_vscard_handle_message(PassthruState *card,
>     const VSCMsgHeader *scr_msg_header);
> 
> What is the project guideline in this case?

I don't know that we have an official guideline, but I've seen enough
code doing that. I've also seen this style:

static void long_func_name(
    parameter one, parameter two)
{
    if (condition) {
        call_some_really_long_name(
            arg1,
            arg2);
    }

where even the first argument is put at an indentation of 4 from the
primary line.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 2/2] CODING_STYLE: indent example code as all others

[Eric Blake]

On 2/19/19 11:38 AM, Philippe Mathieu-Daudé wrote:

>> +    if (a == 1) {
>> +        /* Reads like: "If a equals 1" */
> 
> I guess you found a bug in the documentation :)
> 
> Since 8c06fbdf36bf4d the style asked is:
> 
>     We now require Linux-kernel-style multiline comments:
>         /*
>          * line one
>          * line two
>          */
> 
>> +        do_something();

We only require winged multiline comments when the comment is actually
multiline.  In this case, the comment is a one-liner, and is just fine
as written.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 2/2] CODING_STYLE: indent example code as all others

[Eric Blake]

On 2/18/19 7:31 PM, Wei Yang wrote:
> All the example code are indented with four spaces except this one.
> 
> Fix this by adding four spaces here.
> 
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
>  CODING_STYLE | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 2/2] CODING_STYLE: indent example code as all others

[Philippe Mathieu-Daudé]

On 2/19/19 6:56 PM, Eric Blake wrote:
> On 2/19/19 11:38 AM, Philippe Mathieu-Daudé wrote:
> 
>>> +    if (a == 1) {
>>> +        /* Reads like: "If a equals 1" */
>>
>> I guess you found a bug in the documentation :)
>>
>> Since 8c06fbdf36bf4d the style asked is:
>>
>>     We now require Linux-kernel-style multiline comments:
>>         /*
>>          * line one
>>          * line two
>>          */
>>
>>> +        do_something();
> 
> We only require winged multiline comments when the comment is actually
> multiline.  In this case, the comment is a one-liner, and is just fine
> as written.

Hmm I have a series where I moved code and changed from /* one line */
to the multi-line style, I wonder why and remember checkpatch errors.
Maybe a side-effect from what b94e809d3e fixed.

Anyway, Wei do you mind adding a multi-line example here too?

With/without multi-line example:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!

Phil.

Re: [Qemu-devel] [PATCH 1/2] CODING_STYLE: specify the indent rule for multiline code

[Wei Yang]

On Tue, Feb 19, 2019 at 11:55:04AM -0600, Eric Blake wrote:
>On 2/19/19 11:34 AM, Philippe Mathieu-Daudé wrote:
>
>> 
>> What is still unclear is what to do when a function name is over 60
>> characters (you follow a library/API and can not shorten it), for example:
>> 
>> static void ccid_card_vscard_handle_message(PassthruState *card,
>>     const VSCMsgHeader *scr_msg_header);
>> 
>> What is the project guideline in this case?
>
>I don't know that we have an official guideline, but I've seen enough
>code doing that. I've also seen this style:
>
>static void long_func_name(
>    parameter one, parameter two)
>{
>    if (condition) {
>        call_some_really_long_name(
>            arg1,
>            arg2);
>    }
>
>where even the first argument is put at an indentation of 4 from the
>primary line.

We should put this style in the example too?

>
>-- 
>Eric Blake, Principal Software Engineer
>Red Hat, Inc.           +1-919-301-3226
>Virtualization:  qemu.org | libvirt.org

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH 1/2] CODING_STYLE: specify the indent rule for multiline code

[Wei Yang]

On Tue, Feb 19, 2019 at 11:52:29AM -0600, Eric Blake wrote:
>On 2/18/19 7:31 PM, Wei Yang wrote:
>> We didn't specify the indent rule for multiline code here, which may
>> misleading users. And in current code, the code use different rules.
>> 
>> Add this rule in CODING_STYLE to make sure this is clear to every one.
>> 
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>>  CODING_STYLE | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>> 
>> diff --git a/CODING_STYLE b/CODING_STYLE
>> index ec075dedc4..73f66ca185 100644
>> --- a/CODING_STYLE
>> +++ b/CODING_STYLE
>> @@ -29,6 +29,32 @@ Spaces of course are superior to tabs because:
>>  
>>  Do not leave whitespace dangling off the ends of lines.
>>  
>> +1.1 Multiline Indent
>> +
>> +There are several places where indent is necessary:
>> +
>> + - struct definition
>> + - if/else
>> + - while/for
>> + - function definition & call
>> +
>> +All the above cases apply the same rule: indent with four spaces.
>
>Is this redundant with the earlier statement that "QEMU indents are four
>spaces."?
>
>> +
>> +While the last three case may face another situation: code should spread into
>> +several lines. In this case the rule is align the new line with first
>> +parentheses.
>
>Grammar is awkward - the leading 'while' makes it sound like that you
>have a dependent clause, but then never provide the independent clause.
>Maybe a completely different wording is better:
>
>When breaking up a long line to fit within line widths, align the
>secondary lines just after the opening parenthesis of the first.
>

Sounds better.

>> +
>> +For example:
>> +
>> +    if (a == 1 &&
>> +        b == 2)
>> +
>
>Maybe:
>
>if (a == 1 &&
>    b == 2) {
>
>to match our later recommendations on always using {}.
>

Yep.

>> +    while (a == 1 &&
>> +           b == 2)
>> +
>
>Similar.
>
>> +    do_something(arg1, arg2
>> +                 arg3)
>> +
>
>and here, I'd include the ';' to make it a valid statement.
>

Reasonable.

>>  2. Line width
>>  
>>  Lines should be 80 characters; try not to make them longer.
>> 
>
>-- 
>Eric Blake, Principal Software Engineer
>Red Hat, Inc.           +1-919-301-3226
>Virtualization:  qemu.org | libvirt.org

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH 2/2] CODING_STYLE: indent example code as all others

[Wei Yang]

On Tue, Feb 19, 2019 at 06:38:54PM +0100, Philippe Mathieu-Daudé wrote:
>On 2/19/19 2:31 AM, Wei Yang wrote:
>> All the example code are indented with four spaces except this one.
>> 
>> Fix this by adding four spaces here.
>> 
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> ---
>>  CODING_STYLE | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/CODING_STYLE b/CODING_STYLE
>> index 73f66ca185..27581d80c1 100644
>> --- a/CODING_STYLE
>> +++ b/CODING_STYLE
>> @@ -134,10 +134,10 @@ block to a separate function altogether.
>>  When comparing a variable for (in)equality with a constant, list the
>>  constant on the right, as in:
>>  
>> -if (a == 1) {
>> -    /* Reads like: "If a equals 1" */
>> -    do_something();
>> -}
>> +    if (a == 1) {
>> +        /* Reads like: "If a equals 1" */

I don't get your point.

You mean to put it into multiline comments?

>
>I guess you found a bug in the documentation :)
>
>Since 8c06fbdf36bf4d the style asked is:
>
>    We now require Linux-kernel-style multiline comments:
>        /*
>         * line one
>         * line two
>         */
>
>> +        do_something();
>> +    }
>>  
>>  Rationale: Yoda conditions (as in 'if (1 == a)') are awkward to read.
>>  Besides, good compilers already warn users when '==' is mis-typed as '=',
>> 

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH 2/2] CODING_STYLE: indent example code as all others

[Wei Yang]

On Tue, Feb 19, 2019 at 07:55:31PM +0100, Philippe Mathieu-Daudé wrote:
>On 2/19/19 6:56 PM, Eric Blake wrote:
>> On 2/19/19 11:38 AM, Philippe Mathieu-Daudé wrote:
>> 
>>>> +    if (a == 1) {
>>>> +        /* Reads like: "If a equals 1" */
>>>
>>> I guess you found a bug in the documentation :)
>>>
>>> Since 8c06fbdf36bf4d the style asked is:
>>>
>>>     We now require Linux-kernel-style multiline comments:
>>>         /*
>>>          * line one
>>>          * line two
>>>          */
>>>
>>>> +        do_something();
>> 
>> We only require winged multiline comments when the comment is actually
>> multiline.  In this case, the comment is a one-liner, and is just fine
>> as written.
>
>Hmm I have a series where I moved code and changed from /* one line */
>to the multi-line style, I wonder why and remember checkpatch errors.
>Maybe a side-effect from what b94e809d3e fixed.
>
>Anyway, Wei do you mind adding a multi-line example here too?
>

A multi-line example for multiline comments?

This looks not relavant to this sectioin. I am afraid I will not add
this example here. Sorry for that.

>With/without multi-line example:
>Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
>Thanks!
>
>Phil.

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH 2/2] CODING_STYLE: indent example code as all others

[Philippe Mathieu-Daudé]

On 2/19/19 11:20 PM, Wei Yang wrote:
> On Tue, Feb 19, 2019 at 07:55:31PM +0100, Philippe Mathieu-Daudé wrote:
>> On 2/19/19 6:56 PM, Eric Blake wrote:
>>> On 2/19/19 11:38 AM, Philippe Mathieu-Daudé wrote:
>>>
>>>>> +    if (a == 1) {
>>>>> +        /* Reads like: "If a equals 1" */
>>>>
>>>> I guess you found a bug in the documentation :)
>>>>
>>>> Since 8c06fbdf36bf4d the style asked is:
>>>>
>>>>     We now require Linux-kernel-style multiline comments:
>>>>         /*
>>>>          * line one
>>>>          * line two
>>>>          */
>>>>
>>>>> +        do_something();
>>>
>>> We only require winged multiline comments when the comment is actually
>>> multiline.  In this case, the comment is a one-liner, and is just fine
>>> as written.
>>
>> Hmm I have a series where I moved code and changed from /* one line */
>> to the multi-line style, I wonder why and remember checkpatch errors.
>> Maybe a side-effect from what b94e809d3e fixed.
>>
>> Anyway, Wei do you mind adding a multi-line example here too?
>>
> 
> A multi-line example for multiline comments?
> 
> This looks not relavant to this sectioin. I am afraid I will not add
> this example here. Sorry for that.

No worries, R-b stands.

>> With/without multi-line example:
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> Thanks!
>>
>> Phil.
>