| Message ID | 20200325191830.16553-1-f4bug@amsat.org |
|---|---|
| Headers | show |
| Series | hw: Add missing error-propagation code | expand |
On Wed, Mar 25, 2020 at 8:18 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > 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.maydell@linaro.org> > > Philippe Mathieu-Daud=C3=A9 (12): Hmm is that a git-publish bug? > scripts/coccinelle: Add script to catch missing error_propagate() > calls > hw/arm/bcm2835_peripherals: Add missing error-propagation code > hw/arm/fsl-imx: Add missing error-propagation code > hw/arm/stm32fx05_soc: Add missing error-propagation code > hw/i386/x86: Add missing error-propagation code > hw/dma/xilinx_axidma: Add missing error-propagation code > hw/mips/cps: Add missing error-propagation code > hw/mips/boston: Add missing error-propagation code > hw/mips/mips_malta: Add missing error-propagation code > hw/misc/macio/macio: Add missing error-propagation code > hw/net/xilinx_axienet: Add missing error-propagation code > hw/riscv/sifive_u: Add missing error-propagation code > > ...ect_property_missing_error_propagate.cocci | 58 +++++++++++++++++++ > hw/arm/bcm2835_peripherals.c | 8 +++ > hw/arm/fsl-imx25.c | 8 +++ > hw/arm/fsl-imx6.c | 8 +++ > hw/arm/stm32f205_soc.c | 4 ++ > hw/arm/stm32f405_soc.c | 4 ++ > hw/dma/xilinx_axidma.c | 3 + > hw/i386/x86.c | 4 ++ > hw/mips/boston.c | 17 ++---- > hw/mips/cps.c | 52 +++++++++++++++++ > hw/mips/mips_malta.c | 19 ++++-- > hw/misc/macio/macio.c | 4 ++ > hw/net/xilinx_axienet.c | 3 + > hw/riscv/sifive_u.c | 8 +++ > 14 files changed, 184 insertions(+), 16 deletions(-) > create mode 100644 scripts/coccinelle/object_property_missing_error_propagat= > e.cocci > > --=20 > 2.21.1 >
On Wed, Mar 25, 2020 at 08:20:49PM +0100, Philippe Mathieu-Daudé wrote: > On Wed, Mar 25, 2020 at 8:18 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > > > 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.maydell@linaro.org> > > > > Philippe Mathieu-Daud=C3=A9 (12): > > Hmm is that a git-publish bug? Please run git-publish once more on the same branch and inspect the $TMPDIR/0000-cover-letter.patch email in an editor when git-publish prints "Stopping so you can inspect the patch emails:". It would be interesting to see how your name is formatted, as well as the MIME Content-Type and Content-Transfer-Encoding headers. This information can be compared to the final email sent to the list and/or received by the mailing list. Stefan
Philippe Mathieu-Daudé <f4bug@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.maydell@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?
Hi Markus, Peter. On 3/31/20 3:23 PM, Markus Armbruster wrote: > Philippe Mathieu-Daudé <f4bug@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.maydell@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? 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. Regards, Phil.
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > Hi Markus, Peter. > > On 3/31/20 3:23 PM, Markus Armbruster wrote: >> Philippe Mathieu-Daudé <f4bug@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.maydell@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.
On 3/30/20 11:21 AM, Stefan Hajnoczi wrote: > On Wed, Mar 25, 2020 at 08:20:49PM +0100, Philippe Mathieu-Daudé wrote: >> On Wed, Mar 25, 2020 at 8:18 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >>> >>> 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.maydell@linaro.org> >>> >>> Philippe Mathieu-Daud=C3=A9 (12): >> >> Hmm is that a git-publish bug? > > Please run git-publish once more on the same branch and inspect the > $TMPDIR/0000-cover-letter.patch email in an editor when git-publish > prints "Stopping so you can inspect the patch emails:". > > It would be interesting to see how your name is formatted, as well as > the MIME Content-Type and Content-Transfer-Encoding headers. Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Philippe Mathieu-Daud=C3=A9 (12): > > This information can be compared to the final email sent to the list > and/or received by the mailing list. > > Stefan >
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.maydell@linaro.org> Philippe Mathieu-Daud=C3=A9 (12): scripts/coccinelle: Add script to catch missing error_propagate() calls hw/arm/bcm2835_peripherals: Add missing error-propagation code hw/arm/fsl-imx: Add missing error-propagation code hw/arm/stm32fx05_soc: Add missing error-propagation code hw/i386/x86: Add missing error-propagation code hw/dma/xilinx_axidma: Add missing error-propagation code hw/mips/cps: Add missing error-propagation code hw/mips/boston: Add missing error-propagation code hw/mips/mips_malta: Add missing error-propagation code hw/misc/macio/macio: Add missing error-propagation code hw/net/xilinx_axienet: Add missing error-propagation code hw/riscv/sifive_u: Add missing error-propagation code ...ect_property_missing_error_propagate.cocci | 58 +++++++++++++++++++ hw/arm/bcm2835_peripherals.c | 8 +++ hw/arm/fsl-imx25.c | 8 +++ hw/arm/fsl-imx6.c | 8 +++ hw/arm/stm32f205_soc.c | 4 ++ hw/arm/stm32f405_soc.c | 4 ++ hw/dma/xilinx_axidma.c | 3 + hw/i386/x86.c | 4 ++ hw/mips/boston.c | 17 ++---- hw/mips/cps.c | 52 +++++++++++++++++ hw/mips/mips_malta.c | 19 ++++-- hw/misc/macio/macio.c | 4 ++ hw/net/xilinx_axienet.c | 3 + hw/riscv/sifive_u.c | 8 +++ 14 files changed, 184 insertions(+), 16 deletions(-) create mode 100644 scripts/coccinelle/object_property_missing_error_propagat= e.cocci --=20 2.21.1