From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34249) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WC8sz-0006uF-E5 for qemu-devel@nongnu.org; Sat, 08 Feb 2014 09:28:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WC8ss-0002Lk-3Y for qemu-devel@nongnu.org; Sat, 08 Feb 2014 09:28:01 -0500 Received: from cantor2.suse.de ([195.135.220.15]:44254 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WC8sr-0002La-T4 for qemu-devel@nongnu.org; Sat, 08 Feb 2014 09:27:54 -0500 Message-ID: <52F63EE5.9000501@suse.de> Date: Sat, 08 Feb 2014 15:27:49 +0100 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <20140208092838.epI9YAEbMFnJRt/M%famz@redhat.com> In-Reply-To: <20140208092838.epI9YAEbMFnJRt/M%famz@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] qtest: Don't segfault with invalid -qtest option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: famcool@gmail.com, Anthony Liguori Am 08.02.2014 10:28, schrieb Fam Zheng: > This prints an error message, instead of core dump, when "-qtest" > option value is invalid, e.g.: >=20 > $ ./x86_64-softmmu/qemu-system-x86_64 -qtest unknown > qemu-system-x86_64: Failed to initialize device for qtest: "unknown= " >=20 > Signed-off-by: Fam Zheng > --- > include/sysemu/qtest.h | 2 +- > qtest.c | 10 +++++++++- > vl.c | 5 ++++- > 3 files changed, 14 insertions(+), 3 deletions(-) >=20 > diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h > index 112a661..fc108c7 100644 > --- a/include/sysemu/qtest.h > +++ b/include/sysemu/qtest.h > @@ -24,7 +24,7 @@ static inline bool qtest_enabled(void) > } > =20 > int qtest_init_accel(void); > -void qtest_init(const char *qtest_chrdev, const char *qtest_log); > +int qtest_init(const char *qtest_chrdev, const char *qtest_log); > =20 > static inline int qtest_available(void) > { > diff --git a/qtest.c b/qtest.c > index dcf1301..a037b3b 100644 > --- a/qtest.c > +++ b/qtest.c > @@ -19,6 +19,7 @@ > #include "hw/irq.h" > #include "sysemu/sysemu.h" > #include "sysemu/cpus.h" > +#include "qemu/error-report.h" > =20 > #define MAX_IRQ 256 > =20 > @@ -507,12 +508,18 @@ int qtest_init_accel(void) > return 0; > } > =20 > -void qtest_init(const char *qtest_chrdev, const char *qtest_log) > +int qtest_init(const char *qtest_chrdev, const char *qtest_log) > { > CharDriverState *chr; > =20 > chr =3D qemu_chr_new("qtest", qtest_chrdev, NULL); > =20 > + if (chr =3D=3D NULL) { > + error_report("Failed to initialize device for qtest: \"%s\"", > + qtest_chrdev); > + return -1; > + } > + > qemu_chr_add_handlers(chr, qtest_can_read, qtest_read, qtest_event= , chr); > qemu_chr_fe_set_echo(chr, true); > =20 > @@ -527,4 +534,5 @@ void qtest_init(const char *qtest_chrdev, const cha= r *qtest_log) > } > =20 > qtest_chr =3D chr; > + return 0; > } > diff --git a/vl.c b/vl.c > index 383be1b..97ca823 100644 > --- a/vl.c > +++ b/vl.c > @@ -4078,7 +4078,10 @@ int main(int argc, char **argv, char **envp) > configure_accelerator(); > =20 > if (qtest_chrdev) { > - qtest_init(qtest_chrdev, qtest_log); > + int ret =3D qtest_init(qtest_chrdev, qtest_log); > + if (ret) { > + exit(1); > + } > } > =20 > machine_opts =3D qemu_get_machine_opts(); Why don't you do the exit(1) in qtest_init()? Either that or keep void and return an Error **errp so that you can print it here where the exit(1) is. Otherwise error_report() usage looks good. Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg