[Top][All Lists]

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

Re: [PATCH-for-5.0 00/12] hw: Add missing error-propagation code

From: Philippe Mathieu-Daudé
Subject: Re: [PATCH-for-5.0 00/12] hw: Add missing error-propagation code
Date: Fri, 3 Apr 2020 19:53:01 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0

Hi Markus, Peter.

On 3/31/20 3:23 PM, Markus Armbruster wrote:
Philippe Mathieu-Daudé <address@hidden> writes:

This series is inspired of Peter fix:
"hw/arm/xlnx-zynqmp.c: fix some error-handling code"

Add a cocci script to fix the other places.

Based-on: <address@hidden>

I skimmed the code patches [PATCH 02-12/12], and they look like bug
fixes.  Other reviewers raised a few issues.

I also skimmed the Coccinelle script [PATCH 01].  Peter pointed out a
few things it apparently missed (e.g. in review of PATCH 06+11).
Moreover, the bug pattern applies beyond object_property_set() &
friends.  Perhaps the script can be generalized.  No reason to hold
fixes.  We may want to add suitable notes to the scipt, though.

Can you address the reviews in a v2, so we can get the fixes into -rc1,
due today?

Status on this series (sorry I didn't update earlier).

I addressed Peter's comments, improved/simplified/documented the cocci script (which I split in smaller ones).

Peter suggested other functions can be checked too, not only the "^object_property_set_.*" matches. Indeed, more patches added. Some are big.

Another suggestion is replace in init() 'NULL' Error* final argument by &error_abort. This can be another series on top. However I noticed we can reduce the error_propagate() generated calls in many places, when both init()/realize() exist and the property set is not dependent of parent operation, by moving these calls from realize() to init(). Another cocci script. But to make sense it has to be run previous the "add missing error_propagate" one.

While writing the cocci patches, I had 3 different Coccinelle failures.
Failures not due to a spatch bug, but timeout because C source hard to process. Indeed the C source code was dubious, could get some simplification rewrite. Then spatch could transform them. More patches in the middle.

Now I'm at 47 patches, the reviewed patches at the end of the series.
Too much for RC2. Since I don't think these are critical bugs, but improvements, are you OK to postpone this series to 5.1?

If you think a patch deserves to be in 5.0, point me at it and I can send it ASAP with comments addressed. Else I'll post my series as -for-5.1 soon.



reply via email to

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