[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 7/7] block/curl: code cleanup to comply with
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-block] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style |
Date: |
Wed, 8 Nov 2017 16:01:51 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
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
[Qemu-block] [PATCH v3 4/7] block/sheepdog: code beautification, Jeff Cody, 2017/11/07