| Message ID | 1444637004-20195-15-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 10/12/2015 02:03 AM, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > backends/baum.c | 13 ++++++++----- > include/sysemu/char.h | 3 --- > qemu-char.c | 4 +--- > stubs/Makefile.objs | 1 - > stubs/chr-baum-init.c | 7 ------- > 5 files changed, 9 insertions(+), 19 deletions(-) > delete mode 100644 stubs/chr-baum-init.c > > @@ -586,14 +589,14 @@ CharDriverState *chr_baum_init(void) > > baum->brlapi_fd = brlapi__openConnection(handle, NULL, NULL); > if (baum->brlapi_fd == -1) { > - brlapi_perror("baum_init: brlapi_openConnection"); > + error_setg(errp, "baum_init: brlapi_openConnection"); error_setg() already tracks function name, so you could drop 'baum_init: '. Also, I assume that brlapi_perror() adds additional information to the error message it prints, such as conversion of a brlapi-specific error message in the same manner in which perror() converts errno and in which error_setg_errno() would be used. So I don't know if this conversion is the best. But I'm unfamiliar with brlapi_* in general, to know if there is anything better to use, so I'd rather get a second opinion on this. ' Other than the possibility of the incorrect conversion of (several) error messages, the rest of the patch looks fine to me.
Eric Blake, le Mon 12 Oct 2015 09:30:12 -0600, a écrit : > Also, I assume that brlapi_perror() adds additional information to > the error message it prints, such as conversion of a brlapi-specific > error message in the same manner in which perror() converts errno and in > which error_setg_errno() would be used. Yes. Such additional information is really useful to debug brlapi issues. > So I don't know if this > conversion is the best. But I'm unfamiliar with brlapi_* in general, to > know if there is anything better to use, brlapi_error_t * brlapi_error_location(void); const char * brlapi_strerror(const brlapi_error_t *error); So brlapi_strerror(brlapi_error_location()) will return what you want, i.e. the string that brlapi_error() would have printed. > so I'd rather get a second opinion on this. Cc-ing me would have been useful, "braille" just happened to catch my eye by luck while looking over qemu mails :) Samuel
diff --git a/backends/baum.c b/backends/baum.c index e86a019..281be30 100644 --- a/backends/baum.c +++ b/backends/baum.c @@ -561,7 +561,10 @@ static void baum_close(struct CharDriverState *chr) g_free(baum); } -CharDriverState *chr_baum_init(void) +static CharDriverState *chr_baum_init(const char *id, + ChardevBackend *backend, + ChardevReturn *ret, + Error **errp) { BaumDriverState *baum; CharDriverState *chr; @@ -586,14 +589,14 @@ CharDriverState *chr_baum_init(void) baum->brlapi_fd = brlapi__openConnection(handle, NULL, NULL); if (baum->brlapi_fd == -1) { - brlapi_perror("baum_init: brlapi_openConnection"); + error_setg(errp, "baum_init: brlapi_openConnection"); goto fail_handle; } baum->cellCount_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, baum_cellCount_timer_cb, baum); if (brlapi__getDisplaySize(handle, &baum->x, &baum->y) == -1) { - brlapi_perror("baum_init: brlapi_getDisplaySize"); + error_setg(errp, "baum_init: brlapi_getDisplaySize"); goto fail; } @@ -609,7 +612,7 @@ CharDriverState *chr_baum_init(void) tty = BRLAPI_TTY_DEFAULT; if (brlapi__enterTtyMode(handle, tty, NULL) == -1) { - brlapi_perror("baum_init: brlapi_enterTtyMode"); + error_setg(errp, "baum_init: brlapi_enterTtyMode"); goto fail; } @@ -630,7 +633,7 @@ fail_handle: static void register_types(void) { register_char_driver("braille", CHARDEV_BACKEND_KIND_BRAILLE, NULL, - NULL); + chr_baum_init); } type_init(register_types); diff --git a/include/sysemu/char.h b/include/sysemu/char.h index 2fe8275..77415ec 100644 --- a/include/sysemu/char.h +++ b/include/sysemu/char.h @@ -359,9 +359,6 @@ CharDriverState *qemu_char_get_next_serial(void); /* testdev.c */ CharDriverState *chr_testdev_init(void); -/* baum.c */ -CharDriverState *chr_baum_init(void); - /* console.c */ typedef CharDriverState *(VcHandler)(ChardevVC *vc); diff --git a/qemu-char.c b/qemu-char.c index 8425891..96f40f3 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -4323,11 +4323,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, case CHARDEV_BACKEND_KIND_MSMOUSE: abort(); break; -#ifdef CONFIG_BRLAPI case CHARDEV_BACKEND_KIND_BRAILLE: - chr = chr_baum_init(); + abort(); break; -#endif case CHARDEV_BACKEND_KIND_TESTDEV: chr = chr_testdev_init(); break; diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index 63988ca..8cfa5a2 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -1,6 +1,5 @@ stub-obj-y += arch-query-cpu-def.o stub-obj-y += bdrv-commit-all.o -stub-obj-y += chr-baum-init.o stub-obj-y += chr-testdev.o stub-obj-y += clock-warp.o stub-obj-y += cpu-get-clock.o diff --git a/stubs/chr-baum-init.c b/stubs/chr-baum-init.c deleted file mode 100644 index f5cc6ce..0000000 --- a/stubs/chr-baum-init.c +++ /dev/null @@ -1,7 +0,0 @@ -#include "qemu-common.h" -#include "sysemu/char.h" - -CharDriverState *chr_baum_init(void) -{ - return NULL; -}
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- backends/baum.c | 13 ++++++++----- include/sysemu/char.h | 3 --- qemu-char.c | 4 +--- stubs/Makefile.objs | 1 - stubs/chr-baum-init.c | 7 ------- 5 files changed, 9 insertions(+), 19 deletions(-) delete mode 100644 stubs/chr-baum-init.c