Philippe Mathieu-Daudé <phi...@redhat.com> writes: > Hi Markus, Peter. > > On 3/31/20 3:23 PM, Markus Armbruster wrote: >> Philippe Mathieu-Daudé <f4...@amsat.org> writes: >> >>> This series is inspired of Peter fix: >>> "hw/arm/xlnx-zynqmp.c: fix some error-handling code" >>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg691636.html >>> >>> Add a cocci script to fix the other places. >>> >>> Based-on: <20200324134947.15384-1-peter.mayd...@linaro.org> >> >> 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?
Punting bug fixes to a later release is always kind of sad, but getting that many patches reviewed properly in time for -rc2 feels hopeless, and the bugs old and unthreatending on first glance. > 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. I'm okay with punting the whole series to 5.1. We have quite some Error work pending for 5.1. Beware of conflicts. In particular, I now plan to make object_property_set() & friends return a useful value. This should make your patches a bit smaller.
