diff mbox series

[for-4.0,v9,11/16] qemu_thread: supplement error handling for emulated_realize

Message ID 20181225140449.15786-12-fli@suse.com
State New
Headers show
Series [for-4.0,v9,01/16] Fix segmentation fault when qemu_signal_init fails | expand

Commit Message

Fei Li Dec. 25, 2018, 2:04 p.m. UTC
Utilize the existed errp to propagate the error and do the
corresponding cleanup to replace the temporary &error_abort.

Cc: Cc: Markus Armbruster <armbru@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Fei Li <fli@suse.com>
---
 hw/usb/ccid-card-emulated.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Comments

Markus Armbruster Jan. 7, 2019, 5:31 p.m. UTC | #1
Fei Li <fli@suse.com> writes:

> Utilize the existed errp to propagate the error and do the
> corresponding cleanup to replace the temporary &error_abort.
>
> Cc: Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Fei Li <fli@suse.com>
> ---
>  hw/usb/ccid-card-emulated.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
> index f8ff7ff4a3..9245b4fcad 100644
> --- a/hw/usb/ccid-card-emulated.c
> +++ b/hw/usb/ccid-card-emulated.c
> @@ -32,7 +32,6 @@
>  #include "qemu/thread.h"
>  #include "qemu/main-loop.h"
>  #include "ccid.h"
> -#include "qapi/error.h"
>  
>  #define DPRINTF(card, lvl, fmt, ...) \
>  do {\
> @@ -544,11 +543,15 @@ static void emulated_realize(CCIDCardState *base, Error **errp)
>          error_setg(errp, "%s: failed to initialize vcard", TYPE_EMULATED_CCID);
>          goto out2;
>      }
> -    /* TODO: let the further caller handle the error instead of abort() here */
> -    qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
> -                       card, QEMU_THREAD_JOINABLE, &error_abort);
> -    qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
> -                       card, QEMU_THREAD_JOINABLE, &error_abort);
> +    if (!qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
> +                            card, QEMU_THREAD_JOINABLE, errp)) {
> +        goto out2;
> +    }
> +    if (!qemu_thread_create(&card->apdu_thread_id, "ccid/apdu",
> +                            handle_apdu_thread, card,
> +                            QEMU_THREAD_JOINABLE, errp)) {
> +        goto out2;

You need to stop and join the first thread.

> +    }
>  
>  out2:
>      clean_event_notifier(card);
fei Jan. 9, 2019, 1:21 p.m. UTC | #2
在 2019/1/8 上午1:31, Markus Armbruster 写道:
> Fei Li <fli@suse.com> writes:
>
>> Utilize the existed errp to propagate the error and do the
>> corresponding cleanup to replace the temporary &error_abort.
>>
>> Cc: Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Fei Li <fli@suse.com>
>> ---
>>   hw/usb/ccid-card-emulated.c | 15 +++++++++------
>>   1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
>> index f8ff7ff4a3..9245b4fcad 100644
>> --- a/hw/usb/ccid-card-emulated.c
>> +++ b/hw/usb/ccid-card-emulated.c
>> @@ -32,7 +32,6 @@
>>   #include "qemu/thread.h"
>>   #include "qemu/main-loop.h"
>>   #include "ccid.h"
>> -#include "qapi/error.h"
>>   
>>   #define DPRINTF(card, lvl, fmt, ...) \
>>   do {\
>> @@ -544,11 +543,15 @@ static void emulated_realize(CCIDCardState *base, Error **errp)
>>           error_setg(errp, "%s: failed to initialize vcard", TYPE_EMULATED_CCID);
>>           goto out2;
>>       }
>> -    /* TODO: let the further caller handle the error instead of abort() here */
>> -    qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
>> -                       card, QEMU_THREAD_JOINABLE, &error_abort);
>> -    qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
>> -                       card, QEMU_THREAD_JOINABLE, &error_abort);
>> +    if (!qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
>> +                            card, QEMU_THREAD_JOINABLE, errp)) {
>> +        goto out2;
>> +    }
>> +    if (!qemu_thread_create(&card->apdu_thread_id, "ccid/apdu",
>> +                            handle_apdu_thread, card,
>> +                            QEMU_THREAD_JOINABLE, errp)) {
>> +        goto out2;
> You need to stop and join the first thread.

Thanks for pointing this out!

Have a nice day
Fei
>
>> +    }
>>   
>>   out2:
>>       clean_event_notifier(card);
diff mbox series

Patch

diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index f8ff7ff4a3..9245b4fcad 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -32,7 +32,6 @@ 
 #include "qemu/thread.h"
 #include "qemu/main-loop.h"
 #include "ccid.h"
-#include "qapi/error.h"
 
 #define DPRINTF(card, lvl, fmt, ...) \
 do {\
@@ -544,11 +543,15 @@  static void emulated_realize(CCIDCardState *base, Error **errp)
         error_setg(errp, "%s: failed to initialize vcard", TYPE_EMULATED_CCID);
         goto out2;
     }
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
-                       card, QEMU_THREAD_JOINABLE, &error_abort);
-    qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
-                       card, QEMU_THREAD_JOINABLE, &error_abort);
+    if (!qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
+                            card, QEMU_THREAD_JOINABLE, errp)) {
+        goto out2;
+    }
+    if (!qemu_thread_create(&card->apdu_thread_id, "ccid/apdu",
+                            handle_apdu_thread, card,
+                            QEMU_THREAD_JOINABLE, errp)) {
+        goto out2;
+    }
 
 out2:
     clean_event_notifier(card);