QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: "Markus Armbruster" <armbru@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>
Cc: Paul Burton <pburton@wavecomp.com>,
	Sagar Karandikar <sagark@eecs.berkeley.edu>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
	qemu-devel@nongnu.org,
	Alistair Francis <Alistair.Francis@wdc.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>,
	Richard Henderson <rth@twiddle.net>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Alistair Francis <alistair@alistair23.me>,
	qemu-arm@nongnu.org, David Gibson <david@gibson.dropbear.id.au>,
	qemu-riscv@nongnu.org,
	Bastian Koppelmann <kbastian@mail.uni-paderborn.de>,
	Andrew Baumann <Andrew.Baumann@microsoft.com>,
	Jean-Christophe Dubois <jcd@tribudubois.net>,
	qemu-ppc@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [PATCH-for-5.0 00/12] hw: Add missing error-propagation code
Date: Fri, 3 Apr 2020 19:53:01 +0200
Message-ID: <dbee4e3d-afba-827d-4950-2ac9b99419ab@redhat.com> (raw)
In-Reply-To: <87r1x8vet0.fsf@dusky.pond.sub.org>

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.



  reply index

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-25 19:18 Philippe Mathieu-Daudé
2020-03-25 19:18 ` [PATCH-for-5.0 01/12] scripts/coccinelle: Add script to catch missing error_propagate() calls Philippe Mathieu-Daudé
2020-03-26 21:32   ` Peter Maydell
2020-03-25 19:18 ` [PATCH-for-5.0 02/12] hw/arm/bcm2835_peripherals: Add missing error-propagation code Philippe Mathieu-Daudé
2020-03-26 21:34   ` Peter Maydell
2020-03-25 19:18 ` [PATCH-for-5.0 03/12] hw/arm/fsl-imx: " Philippe Mathieu-Daudé
2020-03-26 21:35   ` Peter Maydell
2020-03-25 19:18 ` [PATCH-for-5.0 04/12] hw/arm/stm32fx05_soc: " Philippe Mathieu-Daudé
2020-03-25 20:51   ` Alistair Francis
2020-03-26 21:45   ` Peter Maydell
2020-03-25 19:18 ` [PATCH-for-5.0 05/12] hw/i386/x86: " Philippe Mathieu-Daudé
2020-03-26 21:38   ` Peter Maydell
2020-03-25 19:18 ` [PATCH-for-5.0 06/12] hw/dma/xilinx_axidma: " Philippe Mathieu-Daudé
2020-03-25 20:52   ` Alistair Francis
2020-03-26 21:46   ` Peter Maydell
2020-03-25 19:18 ` [PATCH-for-5.0 07/12] hw/mips/cps: " Philippe Mathieu-Daudé
2020-03-26 21:43   ` Peter Maydell
2020-03-26 22:48   ` Aleksandar Markovic
2020-03-25 19:18 ` [PATCH-for-5.0 08/12] hw/mips/boston: " Philippe Mathieu-Daudé
2020-03-26 21:47   ` Peter Maydell
2020-03-26 22:50   ` Aleksandar Markovic
2020-03-25 19:18 ` [PATCH-for-5.0 09/12] hw/mips/mips_malta: " Philippe Mathieu-Daudé
2020-03-26 21:49   ` Peter Maydell
2020-03-26 22:49   ` Aleksandar Markovic
2020-03-25 19:18 ` [PATCH-for-5.0 10/12] hw/misc/macio/macio: " Philippe Mathieu-Daudé
2020-03-25 23:55   ` David Gibson
2020-03-26 21:50   ` Peter Maydell
2020-03-25 19:18 ` [PATCH-for-5.0 11/12] hw/net/xilinx_axienet: " Philippe Mathieu-Daudé
2020-03-25 20:52   ` Alistair Francis
2020-03-26 21:51   ` Peter Maydell
2020-03-25 19:18 ` [PATCH-for-5.0 12/12] hw/riscv/sifive_u: " Philippe Mathieu-Daudé
2020-03-25 20:52   ` Alistair Francis
2020-03-26 21:55   ` Peter Maydell
2020-03-31 17:02     ` Philippe Mathieu-Daudé
2020-03-31 17:03       ` Peter Maydell
2020-03-25 19:20 ` [PATCH-for-5.0 00/12] hw: " Philippe Mathieu-Daudé
2020-03-30  9:21   ` Stefan Hajnoczi
2020-04-06 17:47     ` Philippe Mathieu-Daudé
2020-03-31 13:23 ` Markus Armbruster
2020-04-03 17:53   ` Philippe Mathieu-Daudé [this message]
2020-04-04  5:55     ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=dbee4e3d-afba-827d-4950-2ac9b99419ab@redhat.com \
    --to=philmd@redhat.com \
    --cc=Alistair.Francis@wdc.com \
    --cc=Andrew.Baumann@microsoft.com \
    --cc=aleksandar.qemu.devel@gmail.com \
    --cc=aleksandar.rikalo@rt-rk.com \
    --cc=alistair@alistair23.me \
    --cc=armbru@redhat.com \
    --cc=aurelien@aurel32.net \
    --cc=david@gibson.dropbear.id.au \
    --cc=edgar.iglesias@gmail.com \
    --cc=ehabkost@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=jasowang@redhat.com \
    --cc=jcd@tribudubois.net \
    --cc=kbastian@mail.uni-paderborn.de \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=mst@redhat.com \
    --cc=palmer@dabbelt.com \
    --cc=pbonzini@redhat.com \
    --cc=pburton@wavecomp.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=sagark@eecs.berkeley.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git