qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:


From: Blue Swirl
Subject: Re: [Qemu-devel] [PATCH 1/5] Avoid GCC extension ?:
Date: Fri, 13 Jul 2012 14:58:35 +0000

On Fri, Jul 13, 2012 at 9:05 AM, Kevin Wolf <address@hidden> wrote:
> Am 12.07.2012 22:28, schrieb Blue Swirl:
>> On Wed, Jul 11, 2012 at 12:54 PM, Kevin Wolf <address@hidden> wrote:
>>> Am 08.07.2012 14:09, schrieb Andreas Schwab:
>>>> address@hidden writes:
>>>>
>>>>> +    pstrcpy(bs->backing_format, sizeof(bs->backing_format),
>>>>> +            backing_fmt ? backing_file : "");
>>>>
>>>> s/backing_file/backing_fmt/
>>>
>>> Which is why such changes are probably a bad idea. Even more so if they
>>> aren't scripted.
>>
>> Maybe your patches are perfect from day one, but all patches can be
>> buggy. Review should catch some of the bugs, others may be found
>> later. It's not possible to script this because expr1 may have side
>> effects.
>
> No, my patches aren't perfect, each patch is a risk. So all I'm saying
> is that if it ain't broke, don't fix it.

That way leads to stagnated code. If a change is useful and matches
overall architecture, it should be applied.

>
>>> Does this patch improve anything? Last time I checked, qemu only
>>> compiled on gcc anyway.
>>
>> It improves C99 compliance. GCC extensions should not be used unless
>> absolutely required. In the future, it should be possible to compile
>> QEMU with any C compiler, AREG0 patches remove the biggest obstacle.
>
> If this is our goal and we're really close, it might be worth these
> changes. Are you working towards getting a specific compiler to build
> qemu? Can we get a buildbot for this compiler once it works for the
> first time? Because otherwise I'm pretty sure that it will break frequently.

Is it so hard to avoid GCCisms? Perhaps checkpatch.pl (which seems to
be ignored by many people) could be improved to detect this.

I found these with GCC flag -std=c99. There were plenty of other
errors which may or may not be worth fixing. Setting up a buildbot
with this flag should be possible.

>
> Kevin



reply via email to

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