On 06/09/2017 07:26, Thomas Huth wrote:
> You asked for opinions, so here's mine: I agree with you, please do
> *not* add a new QEMU-specific construct here. assert() should be a
> well-known C construct that every programmer should have understood. You
> also need it for other projects. If you haven't understood that it's a
> macro and has side-effects, you should learn it (e.g. during patch
> review), not avoid it, otherwise you'll run into problems in another
> project that is using it again.
> 
> But IMHO we should still try to get rid of wrong usage of assert() in
> the QEMU sources. So maybe we could allow building with NDEBUG one day
> for the brave people who need the extra percent of additional speed.

It's not only about the side effects.  There are a couple cases in
migration (IIRC) where our error propagation is not up to the task, and
failing assertions are used to validate untrusted input.  NDEBUG in that
case can introduce a known vulnerability.

> But as long as we're not there, I think this patch is a good thing to
> avoid wrong expectations.

Agreed.

Paolo

Reply via email to