Re: [Qemu-devel] [PATCH v8 03/13] dump: add API to write header of flatten format

[Laszlo Ersek]

On 02/10/14 23:48, Luiz Capitulino wrote:
> On Mon, 10 Feb 2014 22:57:15 +0100
> Laszlo Ersek <address@hidden> wrote:

>> In short, -Werror and bisection don't mix. They already don't, and we
>> shouldn't expect them to.
> 
> I understand what you're saying, and I don't want people to do needless and
> endless respins, but letting bisect break at will doesn't seem a good option
> either.
> 
> What other options do we have? What's the general QEMU directive in cases like
> this?
> 
>  1. Do what you did in commit 27d59ccd?
> 
>  2. Apply it and let bisect break?
> 
>  3. Drop -Werror?

In commit 27d59ccd, I introduced the create_blob_file() function, in
"tests/i440fx-test.c". The patch was really about nothing else than
introducing this function (which would have made no sense outside of
said C file, so I made it static).

(The new function was to be utilized in the next patch (3bcc77ae).)

Of course gcc whined at 27d59ccd, and I was forced to call
create_blob_file() from main() just to shut it up. This function call
made absolutely no sense. I only added the call in order to convince gcc
that create_blob_file() was not some abandoned, useless function.

Of course, when the tree is built at 27d59ccd, *and* "tests/i440fx-test"
is run, then create_blob_file() runs too, and it *has* an effect. A blob
file is indeed created and unlinked. In other words, in that commit the
utility function is actually exercised, *outside* its intended
scope/context, just to shut up gcc.

To follow suit, Qiao would have to call write_start_flat_header() and
write_end_flat_header() *somewhere* in patch v8 03/13. It would be a
terrible thing to do. It makes absolutely no sense to call these
functions in this patch. And I don't think we could trick gcc with an
"if (0) {...}", because the optimizer would eliminate the call and we'd
be left with the original warning/error.

Summary: in commit 27d59ccd, I did a horrible hack to shut up gcc. It
was only permissible because the hack affected just a test case. It
didn't affect production code that you actually might want to bisect.

The general qemu (and edk2) approach is to rape the code until the
*contemporary* gcc / compiler of choice shuts up. Then, at *real* bisect
time later down the road, with a new compiler release, act surprised
when the tree doesn't build. Then repeat/restart the bisection with
-Werror disabled.

My specific proposal is to test-build the tree at all patches in the
series, *except* at the last one, with -Werror disabled. Then build the
tree at the final patch with -Werror enabled.

Thanks!
Laszlo