From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58437) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZaQof-0004pq-Vg for qemu-devel@nongnu.org; Fri, 11 Sep 2015 12:04:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZaQoa-0001iD-8k for qemu-devel@nongnu.org; Fri, 11 Sep 2015 12:04:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45418) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZaQoa-0001hr-0I for qemu-devel@nongnu.org; Fri, 11 Sep 2015 12:04:40 -0400 References: <1f763d85b66d3767b96afaa59d483005bb3951e5.1441667360.git.crosthwaite.peter@gmail.com> From: Eric Blake Message-ID: <55F2FB92.6020608@redhat.com> Date: Fri, 11 Sep 2015 10:04:34 -0600 MIME-Version: 1.0 In-Reply-To: <1f763d85b66d3767b96afaa59d483005bb3951e5.1441667360.git.crosthwaite.peter@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="182TTwuwolX1b2cIFCiU8lek0XHg57Ee5" Subject: Re: [Qemu-devel] [RFC PATCH v1 05/25] error: Add error prefix API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, armbru@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --182TTwuwolX1b2cIFCiU8lek0XHg57Ee5 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 09/10/2015 11:33 PM, Peter Crosthwaite wrote: > Add an API to prefix an already set error with a caller-centric > message. >=20 > If multiple errors are set, all are prefixed individually. >=20 Might be nice to rebase your series to add this first, prior to multi-error support. (That is, while prefixing multiple errors requires multi-error support, adding caller-centric prefix might be useful even now without multi-error). > Signed-off-by: Peter Crosthwaite > --- >=20 > include/qapi/error.h | 6 ++++++ > util/error.c | 26 ++++++++++++++++++++++++++ > 2 files changed, 32 insertions(+) >=20 > diff --git a/include/qapi/error.h b/include/qapi/error.h > index f44c451..b25c72f 100644 > --- a/include/qapi/error.h > +++ b/include/qapi/error.h > @@ -78,6 +78,12 @@ ErrorClass error_get_class(const Error *err); > Error *error_copy(const Error *err); > =20 > /** > + * Prefix an error message with a formatted string. > + */ > + > +void error_prefix(Error *err, const char *fmt, ...); The string is basically only adding details for human consumption. > +++ b/util/error.c > @@ -19,6 +19,7 @@ struct Error > char *msg; > ErrorClass err_class; > struct Error *next; > + bool prefixed; > }; I'm not sure I follow why this field is needed. > =20 > Error *error_abort; > @@ -142,6 +143,26 @@ const char *error_get_pretty(Error *err) > return err->msg; > } > =20 > +void error_prefix(Error *err, const char *fmt, ...) { Brace on its own line. > + char *msg; > + char *fmt_full; > + va_list ap; > + > + if (!err || err->prefixed) { > + return; > + } > + err->prefixed =3D true; > + > + msg =3D err->msg; > + fmt_full =3D g_strdup_printf("%s%%s", fmt); > + > + va_start(ap, fmt); > + err->msg =3D g_strdup_printf(fmt_full, ap, msg); I don't think that is portable. Passing va_list as an argument to plain printf just isn't going to do what you think. (There has been a request to add recursive printf via %r, http://austingroupbugs.net/view.php?id=3D800, but glibc does not support the extension yet) It's not to say that you can't chain, just that you can't chain like this. I don't know if using GString internally to error would make the matter any easier (but one of the patches that will probably land before yours, and thus affect your need to rebase, is mine which adds use of GString for adding a human-readable SUFFIX rather than prefix: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02818.html) > + va_end(ap); > + g_free(fmt_full); > + g_free(msg); > +} > + > void error_report_err(Error *err) > { > error_report("%s", error_get_pretty(err)); > @@ -170,6 +191,11 @@ void error_propagate(Error **dst_errp, Error *loca= l_err) > =20 > *dst_errp =3D local_err; > for (i =3D local_err; i; i =3D i->next) { > + /* Propagation implies that the caller is no longer the ow= ner of the > + * error. Therefore reset prefixes, so higher level handle= rs can > + * prefix again. > + */ > + i->prefixed =3D false; I'm still not quite sure I buy the semantics of this flag. > dst_errp =3D &i->next; > } > *dst_errp =3D old_dst_err; >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --182TTwuwolX1b2cIFCiU8lek0XHg57Ee5 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJV8vuSAAoJEKeha0olJ0NqbzkIAJyl0dvIXXHmK1oen2Vj0G36 vyhUlRz6Uctc5uMFxxQQDkmVmilJUhQ31JA7CuQIafmKmkI0um9g/hjMMPqS6Mpa 3olmL5Lsi9mwRSxqi/Qin7ynPcgtedwMPl9hesKdpNj+cBQ2gslyMikGtNSkU6aL WQmXALTHUkgNq+EKJV8eDOo5XvkSxx+3mXCN5muotXc+dbr644Br93Mf94j4cnNF KpmlKcplogcZp4iA3lIn/sV0axhBti+7s5KqRyRHSTR0shR3lTvMcqFf+MLs+z0n 6uk3IkdnxNkhprGEbjY6sPEa9doWA75xoB3IMxBnO1NcZ+ZUI1UTPrTWVM1Mw6o= =wdyu -----END PGP SIGNATURE----- --182TTwuwolX1b2cIFCiU8lek0XHg57Ee5--