From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55173) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOuq3-0000YC-0r for qemu-devel@nongnu.org; Thu, 28 Jan 2016 17:14:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aOupy-0001MN-Ug for qemu-devel@nongnu.org; Thu, 28 Jan 2016 17:14:50 -0500 Received: from roura.ac.upc.edu ([147.83.33.10]:42800 helo=roura.ac.upc.es) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOupy-0001M6-IV for qemu-devel@nongnu.org; Thu, 28 Jan 2016 17:14:46 -0500 From: =?utf-8?Q?Llu=C3=ADs_Vilanova?= References: <145401727502.31479.11571397180565966513.stgit@localhost> <145401730295.31479.7043845714018218355.stgit@localhost> <56AA8D2C.5050403@redhat.com> Date: Thu, 28 Jan 2016 23:14:42 +0100 In-Reply-To: <56AA8D2C.5050403@redhat.com> (Eric Blake's message of "Thu, 28 Jan 2016 14:50:36 -0700") Message-ID: <87bn85tkq5.fsf@fimbulvetr.bsc.es> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 5/5] doc: Introduce coding style for errors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Stefan Hajnoczi , Thomas Huth , qemu-devel@nongnu.org, Markus Armbruster , "Dr . David Alan Gilbert" Eric Blake writes: > On 01/28/2016 02:41 PM, Llu=C3=ADs Vilanova wrote: >> Gives some general guidelines for reporting errors in QEMU. >>=20 >> Signed-off-by: Llu=C3=ADs Vilanova >> --- >> HACKING | 33 +++++++++++++++++++++++++++++++++ >> 1 file changed, 33 insertions(+) >>=20 >> diff --git a/HACKING b/HACKING >> index 12fbc8a..f5783d4 100644 >> --- a/HACKING >> +++ b/HACKING >> @@ -157,3 +157,36 @@ painful. These are: >> * you may assume that integers are 2s complement representation >> * you may assume that right shift of a signed integer duplicates >> the sign bit (ie it is an arithmetic shift, not a logical shift) >> + >> +7. Error reporting >> + >> +QEMU provides various mechanisms for reporting errors using a uniform f= ormat, >> +ensuring the user will receive them (e.g., shown in QMP when necessary)= . You >> +should use one of these mechanisms instead of manually reporting them (= i.e., do >> +not use 'printf()', 'exit()' or 'abort()'). > abort() for unreachable code may be okay, but I'm not sure how to word > that. Maybe "avoid use 'printf()' or 'exit()', and minimize use of > 'abort()' situations that should be unreachable code". Hmmm. I was thinking it'd be more informative to always use error_report_ab= ort() instead of abort: * The program name is shown (great for multi-app logs) * The aborting location is shown (a bit useful to quickly see where it comes from) * A message can be provided I think the message should be optional, since error_report_abort() already provides more information than plain abort(). But if that seems unreasonable, I can reword it as: QEMU provides various mechanisms for reporting errors using a uniform for= mat, ensuring the user will receive them (e.g., shown in QMP when necessary). = You should use one of these mechanisms instead of manually reporting them; i.= e., do not use 'printf()' nor 'exit()', and minimize the use of 'abort()' to situations where code should be unreachable and an error message does not= make sense. > May be worth mentioning that if the user can trigger it (command line, > hotplug, etc) then we want fatal; if it represents a programming bug > that the user should not be able to trigger, then abort is okay. So true. I'll add these. >> + >> +7.1. Simple error messages >> + >> +The 'error_report*()' functions in "include/qemu/error-report.h" will >> +immediately report error messages to the user. >> + >> +WARNING: Do *not* use 'error_report_fatal()' or 'error_report_abort()' = for >> +errors that are (or can be) triggered by guest code (e.g., some unimpli= mented > s/unimplimented/unimplemented/ Fixed in v5 (sorry I forgot about it). >> +corner case in guest code translation or device code). Otherwise that c= an be >> +abused by guest code to terminate QEMU. Instead, you should use >> +'error_report()'. >> + >> +7.2. Errors in user inputs >> + >> +The 'loc_*()' functions in "include/qemu/error-report.h" will extend the >> +messages from 'error_report*()' with references to locations in inputs = provided >> +by the user (e.g., command line arguments or configuration files). >> + >> +7.3. More complex error management >> + >> +The functions in "include/qapi/error.h" can be used to accumulate error= messages >> +in an 'Error' object, which can be propagated up the call chain where i= t is >> +finally reported. >> + >> +WARNING: The special 'error_fatal' and 'error_abort' objects follow the= same >> +constrains as the 'error_report_fatal' and 'error_report_abort' functio= ns. > s/constrains/constraints/ Will add. Thanks, Lluis