From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39569) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSipT-0000xt-Ei for qemu-devel@nongnu.org; Thu, 28 Jul 2016 06:46:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bSipO-0000Hu-IV for qemu-devel@nongnu.org; Thu, 28 Jul 2016 06:46:14 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:62199) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSipO-0000Gj-8U for qemu-devel@nongnu.org; Thu, 28 Jul 2016 06:46:10 -0400 Received: from pps.filterd (m0098404.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u6SAiIdN109734 for ; Thu, 28 Jul 2016 06:46:09 -0400 Received: from e06smtp15.uk.ibm.com (e06smtp15.uk.ibm.com [195.75.94.111]) by mx0a-001b2d01.pphosted.com with ESMTP id 24e1hagsg7-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 28 Jul 2016 06:46:09 -0400 Received: from localhost by e06smtp15.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 28 Jul 2016 11:46:06 +0100 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id 7EB581B08023 for ; Thu, 28 Jul 2016 11:47:32 +0100 (BST) Received: from d06av03.portsmouth.uk.ibm.com (d06av03.portsmouth.uk.ibm.com [9.149.37.213]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u6SAk4wl21233716 for ; Thu, 28 Jul 2016 10:46:04 GMT Received: from d06av03.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u6SAk3Yd011901 for ; Thu, 28 Jul 2016 04:46:04 -0600 References: <1469611466-31574-1-git-send-email-silbe@linux.vnet.ibm.com> <57991CC1.40003@redhat.com> <87twfaysit.fsf@dusky.pond.sub.org> From: Halil Pasic Date: Thu, 28 Jul 2016 12:46:03 +0200 MIME-Version: 1.0 In-Reply-To: <87twfaysit.fsf@dusky.pond.sub.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Message-Id: <5799E26B.7090408@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH] error: error_setg_errno(): errno gets preserved List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , Eric Blake Cc: Sascha Silbe , qemu-devel@nongnu.org, Kevin Wolf , Michael Roth , Cornelia Huck , Max Reitz On 07/28/2016 12:19 PM, Markus Armbruster wrote: > Eric Blake writes: > >> On 07/27/2016 03:24 AM, Sascha Silbe wrote: >>> C11 allows errno to be clobbered by pretty much any library function >>> call, so in general callers need to take care to save errno before >>> calling other functions. >>> >>> However, for error reporting functions this is rather awkward and can >>> make the code on the caller side more complicated than >>> necessary. error_setg_errno() already takes care of preserving errno >>> and some functions rely on that, so just promise that we continue to >>> do so in the future. >>> >>> Signed-off-by: Sascha Silbe >>> --- >>> >>> Alternative approach to "error: error_setg_errno(): errno may be >>> clobbered" [1]. >> >> I like this alternative better. >> >> >>> +++ b/include/qapi/error.h >>> @@ -170,6 +170,9 @@ void error_setg_internal(Error **errp, >>> * Just like error_setg(), with @os_error info added to the message. >>> * If @os_error is non-zero, ": " + strerror(os_error) is appended to >>> * the human-readable error message. >>> + * >>> + * The value of errno (which usually can get clobbered by almost any >>> + * function call) will be preserved. >>> */ >>> #define error_setg_errno(errp, os_error, fmt, ...) \ >>> error_setg_errno_internal((errp), __FILE__, __LINE__, __func__, \ >> >> Do we need/want to make the guarantee of preserving errno across any of >> the other functions and macros declared in error.h? > > I guess we should for the ones that preserve errno, to make that > preservation actually useful. These are: error_setv(), > error_setg_errno_internal(), error_append_hint(). Indirectly: > error_set_internal(), error_set(), error_setg_internal(), error_setg(), > error_setg_file_open_internal(), error_setg_file_open(), possibly > error_setg_win32_internal() and error_setg_win32(). > The implementation of preserve errno seems inconsistent to me. The function error_setv is static, and I guess it is supposed to provide this indirect errno preservation and is used for both error_setg und error_setg_errno, yet error_setg_ errno_internal does extra save-restore itself while error_setg_iternal relies on 'indirect', what is not OK in my opinion. As Sascha pointed out, in C11 any library functions may change errno unless explicitly told otherwise for the particular function. Since start_va and end_va has nothing on preserving errno it is guaranteed by the standard that they persevere errno, and we should assume they don't. I could prepare a patch for this. Should I? Halil