From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47446) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TtN42-0004kW-DN for qemu-devel@nongnu.org; Thu, 10 Jan 2013 13:41:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TtN3y-0003hj-Hg for qemu-devel@nongnu.org; Thu, 10 Jan 2013 13:41:18 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37962) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TtN3y-0003ew-AN for qemu-devel@nongnu.org; Thu, 10 Jan 2013 13:41:14 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r0AIes1q018819 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 10 Jan 2013 13:41:03 -0500 Message-ID: <50EF0B29.1040801@redhat.com> Date: Thu, 10 Jan 2013 11:40:41 -0700 From: Eric Blake MIME-Version: 1.0 References: <1357827786-14624-1-git-send-email-kraxel@redhat.com> <1357827786-14624-5-git-send-email-kraxel@redhat.com> In-Reply-To: <1357827786-14624-5-git-send-email-kraxel@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enigA1FAA3CE5114A0AF25404829" Subject: Re: [Qemu-devel] [PATCH v2 04/10] chardev: add qmp hotplug commands, with null chardev support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigA1FAA3CE5114A0AF25404829 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 01/10/2013 07:23 AM, Gerd Hoffmann wrote: > Add chardev-add and chardev-remove qmp commands. Hotplugging > a null chardev is supported for now, more will be added later. >=20 > Signed-off-by: Gerd Hoffmann > --- > + > +## > +# @ChardevReturn: > +# > +# Return infos about the chardev backend just created. s/infos/info/ or maybe s/infos/details/ > +ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend= , > + Error **errp) > +{ > + ChardevReturn *ret =3D g_new0(ChardevReturn, 1); > + CharDriverState *chr =3D NULL; > + Should you add: chr =3D qemu_chr_find(id); if (chr) { error_setg(errp, "Chardev '%s' already exists", id); return NULL; } > + switch (backend->kind) { > + case CHARDEV_BACKEND_KIND_NULL: > + chr =3D qemu_chr_open_null(NULL); > + break; > + default: > + error_setg(errp, "unknown chardev backend (%d)", backend->kind= ); > + break; > + } > + > + if (chr =3D=3D NULL && !error_is_set(errp)) { > + error_setg(errp, "Failed to create chardev"); > + } > + if (chr) { > + chr->label =3D g_strdup(id); > + chr->avail_connections =3D 1; > + QTAILQ_INSERT_TAIL(&chardevs, chr, next); > + } > + return ret; Do you really want to be returning a successful object even when the error is set, or should you start with ret =3D NULL, and only do the g_new0(ChardevReturn, 1) on success? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enigA1FAA3CE5114A0AF25404829 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQEcBAEBCAAGBQJQ7wspAAoJEKeha0olJ0NqKWsH/2/nLYZnMe1HTDEdrDZyHc5l ROOXll0xdNhq3WizEKJobsWzP2l+b3r9EGfPCHTIWoWk1LT/bhHG/Mm+3IVP97vE axiMGT3OhnWQnnOzL6wj+haFvHbo0EA1wWNp+Y4lM8hnJmyJj0IYx/rizRidEy6j f2yyZIRgYDgZ5xDI39QV7bfDleAnmJd/xTdKD4nFER/uiMA0uQ9U7nVCw3V53O7N Wk2b55WsBPB5MvrlGf/Y+qawhurNxIVOc2gXwHLQ/E9W+a56RWuzrhsnWxVenubE kaku/37xKebBro6asx4rTZgrDH/Huxf6vKXSK59wxHbtYh/0nNFSwlV83z+IA24= =EJsy -----END PGP SIGNATURE----- --------------enigA1FAA3CE5114A0AF25404829--