From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59808) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fRFYw-0004xe-4S for qemu-devel@nongnu.org; Fri, 08 Jun 2018 07:28:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fRFYs-0005EK-5t for qemu-devel@nongnu.org; Fri, 08 Jun 2018 07:28:10 -0400 References: <20180607165218.9558-1-david@redhat.com> <20180607165218.9558-3-david@redhat.com> <20180608093406.30c878ec@bahia.lan> <4f518685-fcdb-7c3a-35d3-5a26374e0604@redhat.com> <20180608094657.4043502f@bahia.lan> <431d4fe5-5bfe-1ab3-00b5-610038a7a9ea@redhat.com> <2b431d32-9172-0517-4717-7ce50355b8db@redhat.com> <20180608103943.1d7e72ee@redhat.com> <673ffa52-1e24-5d6c-5d6a-927d3df79273@redhat.com> <20180608110612.18ac915c@redhat.com> <9eaefde2-b8aa-9072-610b-4236ce5c3913@redhat.com> <20180608125233.46fc187a@bahia.lan> From: David Hildenbrand Message-ID: Date: Fri, 8 Jun 2018 13:28:01 +0200 MIME-Version: 1.0 In-Reply-To: <20180608125233.46fc187a@bahia.lan> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [qemu-s390x] [PATCH v1 2/8] spapr: no need to verify the node List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz Cc: Igor Mammedov , Thomas Huth , Cornelia Huck , Eduardo Habkost , Peter Crosthwaite , "Michael S . Tsirkin" , qemu-devel@nongnu.org, Alexander Graf , Christian Borntraeger , qemu-s390x@nongnu.org, qemu-ppc@nongnu.org, Paolo Bonzini , David Gibson , Richard Henderson On 08.06.2018 12:52, Greg Kurz wrote: > On Fri, 8 Jun 2018 11:24:51 +0200 > David Hildenbrand wrote: > >>>>>> +1 for error_abort, even if it takes another line. >>>>> +1 for error_abort >>>>> call shouldn't fail, but if does it won't be silently ignored >>>>> and introduce undefined behavior. >>>> >>>> Maybe we should fix the others that pass in NULL. >>>> >>>> (no, not me :D - I'm already busy with your requested pre_plug handling) >>> Add it to wiki page for bite sized tasks? >> >> Done. >> >> > > FWIW, I've also added a line to check and possibly fix places where we do > 'if (*errp)', which would cause QEMU to crash if the caller passes NULL. > > $ git grep 'if (\*errp)' > hmp.c: if (*errp) { > hw/ipmi/isa_ipmi_bt.c: if (*errp) > hw/ipmi/isa_ipmi_kcs.c: if (*errp) > hw/mem/memory-device.c: if (*errp) { > hw/mem/memory-device.c: if (*errp) { > hw/ppc/spapr.c: if (*errp) { > hw/s390x/event-facility.c: if (*errp) { > include/qapi/error.h: * if (*errp) { // WRONG! > qga/commands-posix.c: if (*errp) { > target/s390x/cpu_models.c: if (*errp) { > target/s390x/cpu_models.c: if (*errp) { > target/s390x/cpu_models.c: if (*errp) { > target/s390x/cpu_models.c: if (*errp) { > target/s390x/cpu_models.c: if (*errp) { > target/s390x/cpu_models.c: if (*errp) { > target/s390x/cpu_models.c: if (*errp) { > target/s390x/cpu_models.c: if (*errp) { > target/s390x/cpu_models.c: if (*errp) { > target/s390x/cpu_models.c: if (*errp) { > target/s390x/cpu_models.c: if (*errp) { > target/s390x/cpu_models.c: if (*errp) { > target/s390x/cpu_models.c: if (*errp) { > tests/test-crypto-tlscredsx509.c: if (*errp) { > tests/test-io-channel-tls.c: if (*errp) { > I think the more important part is actually looking out for people that use NULL instead of error_abort. This way we won't silently ignore errors. -- Thanks, David / dhildenb