Re: [Qemu-devel] [Qemu-block] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style

[Paolo Bonzini]

On 08/11/2017 11:47, Darren Kenny wrote:
>>
>>
>> -// #define DEBUG_CURL
>> -// #define DEBUG_VERBOSE
>> +/*
>> + #define DEBUG_CURL
>> + #define DEBUG_VERBOSE
>> +*/
> 
> Is it not more common to use the style:
> 
>    /*
>     * text
>     */
> 
> This is visible in almost every file at the top, where the Copyright
> and License is.

The most common style in QEMU is probably

   /* text
    * more text
    */

But for DEBUG_* macros I think // are appropriate.  checkpatch should
still warn about them because DEBUG_* macros should be replaced by
tracepoints; but unless we do that we should keep line comments.

>>
>> -    CURLState *s = ((CURLState*)opaque);
>> +    CURLState *s = opaque;
> 
> Not sure about this one - while it may not be strictly necessary to
> do the casting, from a readability point of view it is helpful to
> see the cast - possibly with the outer brackets removed:
> 
>      CURLState *s = (CURLState*)opaque;

I think the most common idiom here is to omit the cast.

>> -    // In case we have the requested data already (e.g. read-ahead),
>> -    // we can just call the callback and be done.
>> +    /* In case we have the requested data already (e.g. read-ahead),
>> +       we can just call the callback and be done. */
> 
> Please don't do a multi-line comment like this, it is much more
> obvious that the second line is a comment if you use the more normal
> format of as outlined earlier by me. 

Agreed.

Paolo