On 08.06.2018 12:52, Greg Kurz wrote:
> On Fri, 8 Jun 2018 11:24:51 +0200
> David Hildenbrand <da...@redhat.com> 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
