From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35673) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9btm-00023W-AF for qemu-devel@nongnu.org; Thu, 17 Dec 2015 11:59:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a9bti-0001q5-Go for qemu-devel@nongnu.org; Thu, 17 Dec 2015 11:59:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58067) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9bti-0001pw-9W for qemu-devel@nongnu.org; Thu, 17 Dec 2015 11:59:22 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id D85C48A14E for ; Thu, 17 Dec 2015 16:59:21 +0000 (UTC) References: <1450371004-26866-1-git-send-email-armbru@redhat.com> <1450371004-26866-6-git-send-email-armbru@redhat.com> From: Eric Blake Message-ID: <5672E9E5.70004@redhat.com> Date: Thu, 17 Dec 2015 09:59:17 -0700 MIME-Version: 1.0 In-Reply-To: <1450371004-26866-6-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="UJDHfgLWijhnu4PxdgMsD0h9EguQUIeI3" Subject: Re: [Qemu-devel] [PATCH v2 05/23] error: Improve documentation around error_append_hint() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --UJDHfgLWijhnu4PxdgMsD0h9EguQUIeI3 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 12/17/2015 09:49 AM, Markus Armbruster wrote: > While there, tighten its assertion. >=20 > Signed-off-by: Markus Armbruster > --- > include/qapi/error.h | 19 +++++++++++++++++-- > util/error.c | 2 +- > 2 files changed, 18 insertions(+), 3 deletions(-) >=20 > diff --git a/include/qapi/error.h b/include/qapi/error.h > index b2362a5..048d947 100644 > --- a/include/qapi/error.h > +++ b/include/qapi/error.h > @@ -18,6 +18,15 @@ > * Create an error: > * error_setg(&err, "situation normal, all fouled up"); > * > + * Create an error and add additional explanation: > + * error_setg(&err, "invalid quark"); > + * error_append_hint(&err, "Valid quarks are up, down, strange, " > + * "charm, top, bottom\n"); Do we want to allow/encourage a trailing dot in the hint? I'm fine if we don't - but once we document its usage, we should probably then be consistent with what we document; qemu-option.c used a trailing dot, qdev-monitor.c did not. > + * > + * Do *not* contract this to > + * error_setg(&err, "invalid quark\n" > + * "Valid quarks are up, down, strange, charm, top, bot= tom"); > + * > * Report an error to stderr: > * error_report_err(err); > * This frees the error object. > @@ -26,6 +35,7 @@ > * const char *msg =3D error_get_pretty(err); > * do with msg what needs to be done... > * error_free(err); > + * Note that this loses hints added with error_append_hint(). > * > * Handle an error without reporting it (just for completeness): > * error_free(err); > @@ -128,6 +138,7 @@ ErrorClass error_get_class(const Error *err); > * If @errp is anything else, *@errp must be NULL. > * The new error's class is ERROR_CLASS_GENERIC_ERROR, and its > * human-readable error message is made from printf-style @fmt, ... > + * The resulting message should not contain newlines. Should we also discourage trailing punctuation? Should we also mention no need for leading 'error: ' prefix? > */ > #define error_setg(errp, fmt, ...) \ > error_setg_internal((errp), __FILE__, __LINE__, __func__, \ > @@ -184,7 +195,11 @@ void error_propagate(Error **dst_errp, Error *loca= l_err); > =20 > /** > * Append a printf-style human-readable explanation to an existing err= or. > - * May be called multiple times, and safe if @errp is NULL. > + * @errp may be NULL, but neither &error_fatal nor &error_abort. As a native speaker, 'not' sounds better than 'neither' in that sentence. But I think both choices are grammatically correct. > + * Trivially the case if you call it only after error_setg() or > + * error_propagate(). > + * May be called multiple times. The resulting hint should end with a= > + * newline. > */ > void error_append_hint(Error **errp, const char *fmt, ...) > GCC_FMT_ATTR(2, 3); > @@ -218,7 +233,7 @@ void error_free_or_abort(Error **errp); > /* > * Convenience function to error_report() and free @err. > */ > -void error_report_err(Error *); > +void error_report_err(Error *err); > =20 > /* > * Just like error_setg(), except you get to specify the error class. > diff --git a/util/error.c b/util/error.c > index 9b27c45..ebfb74b 100644 > --- a/util/error.c > +++ b/util/error.c > @@ -132,7 +132,7 @@ void error_append_hint(Error **errp, const char *fm= t, ...) > return; > } > err =3D *errp; > - assert(err && errp !=3D &error_abort); > + assert(err && errp !=3D &error_abort && errp !=3D &error_fatal); Otherwise looks reasonable. > =20 > if (!err->hint) { > err->hint =3D g_string_new(NULL); >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --UJDHfgLWijhnu4PxdgMsD0h9EguQUIeI3 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/ iQEcBAEBCAAGBQJWcunlAAoJEKeha0olJ0NqCzgH/RNYWc4vD7n66xp0txAJtidU C1NLVTwr681c61nG/6sRcDJmjDGn4TWZSQjxd7dBX7gRqCPUwB8g7XHTVrJFwagn eh+YIJ6qSiD7XTcY3uamT+lXLfDUJGIjVAFLuqomp8FeOEN+J/An40odQYkNTeZ0 9ZYkEz/FAK/Q6md2CthSBM8kuTinvaadK7FiXK8s1eUhPspoBe4xrVU2/G91FmP5 BWdSuGXywfLIVjxDs+fb5S9+9RlE56Z9AWPdKDEAbZTfpkjpdW690ZtICbpdvBZf b0pKHwyvkX9gXxzK34Nk1/SPMw2+62An8ghY2xqrKcsg986t9kP3ravK5f/79HU= =PEQp -----END PGP SIGNATURE----- --UJDHfgLWijhnu4PxdgMsD0h9EguQUIeI3--