qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PULL 00/17] Misc changes for QEMU 2.10-rc1 (?)


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PULL 00/17] Misc changes for QEMU 2.10-rc1 (?)
Date: Tue, 1 Aug 2017 19:17:24 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1

On 01/08/2017 19:10, Peter Maydell wrote:
> On 1 August 2017 at 17:50, Paolo Bonzini <address@hidden> wrote:
>> On 01/08/2017 18:48, address@hidden wrote:
>>> ERROR: space prohibited before that '++' (ctx:WxB)
>>> #78: FILE: hw/bt/sdp.c:741:
>>> +        data[len ++] = attribute_id >> 8;
>>>                   ^
>>>
>>> ERROR: space prohibited before that '++' (ctx:WxB)
>>> #79: FILE: hw/bt/sdp.c:742:
>>> +        data[len ++] = attribute_id & 0xff;
>>
>> This is the preexisting Bluetooth code... I didn't change the space,
>> should I have done that?
> 
> Judgement call -- I usually fix up existing errors if I'm touching
> a bit of code anyway, unless it's a whitespace-only change or a
> pure code-motion patch.

Me too.  In this case however the code is pretty much untouched so it's
unlikely that it would become consistent one day (and I suspect no one
wants to get on git blame for bluetooth emulation :)).

>>> ERROR: space required before the open parenthesis '('
>>> #73: FILE: tests/rtc-test.c:344:
>>> +    } while(0)
>>
>> This seems to be more common than "while (0)" inside macros, should we
>> allow it in checkpatch.pl?
> 
> Overall the space is much more common: 551 examples with the
> space vs 90 without; so I don't think a relaxation of checkpatch
> is particularly justified. I don't think macros need to be any
> different from the rest of our code on things like spacing.

Ok, for this patch I kept it consistent within the file, but for 2.11 we
can change everything to "while (0)".

Paolo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]