From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53687) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XmF0c-0006Ob-Lo for qemu-devel@nongnu.org; Wed, 05 Nov 2014 23:49:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XmF0T-0004Ab-2p for qemu-devel@nongnu.org; Wed, 05 Nov 2014 23:49:22 -0500 Received: from mail-pa0-x22b.google.com ([2607:f8b0:400e:c03::22b]:47798) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XmF0S-0004AE-SE for qemu-devel@nongnu.org; Wed, 05 Nov 2014 23:49:13 -0500 Received: by mail-pa0-f43.google.com with SMTP id eu11so472443pac.2 for ; Wed, 05 Nov 2014 20:49:11 -0800 (PST) Date: Thu, 6 Nov 2014 12:49:03 +0800 From: Amos Kong Message-ID: <20141106044903.GA8764@air.redhat.com> References: <1415174992-13246-1-git-send-email-syeon.hwang@samsung.com> <5459DBE1.30509@redhat.com> <545A0288.4010905@gnu.org> <545A05CC.4050109@redhat.com> <545A07C6.1020001@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ibTvN161/egqYuK8" Content-Disposition: inline In-Reply-To: <545A07C6.1020001@redhat.com> Subject: Re: [Qemu-devel] [PATCH] error: fixed error_set_errno() to deal with a negative type of os_error. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Paolo Bonzini , paolo.bonzini@gmail.com, armbru@redhat.com, qemu-devel@nongnu.org, SeokYeon Hwang , Max Reitz --ibTvN161/egqYuK8 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 05, 2014 at 12:19:34PM +0100, Eric Blake wrote: > On 11/05/2014 12:11 PM, Max Reitz wrote: >=20 > >>>> + err->msg =3D g_strdup_printf("%s: %s", msg1, > >>>> strerror(abs(os_errno))); >=20 > >> I don't, we really should fix the callers. > >=20 > > Of course I understand, but this patch doesn't make matters worse, as > > long as there are not systems which have negative values for errno >=20 > POSIX requires all defined errno values to be positive; negative errno > values are unambiguous as values that will cause strerror() to have to > generate a message about an unknown value. >=20 > > (which I think we generally assume not to exist throughout qemu). That's > > why I'm fine with it. We should fix the callers but I don't see why we > > shouldn't apply this patch as well. >=20 > This patch is a bandaid; it makes it harder to find callers that need to > be fixed. I'd almost argue the exact opposite - add an assert(os_errno > > 0). Then we'd loudly break on broken callers, making them easier to fi= nd. Agree! use an assert to teach the caller ;) > > A similar issue already came up and led to commit b276d2499, where > > callers of error_setg_errno() assumed that it would not clobber errno, > > so we fixed some of the callers but also applied that commit which just > > saves errno because there's no reason not to. >=20 > If we're willing to accept the convenience so that callers can be lazy, > then I like this patch. If we want to fix bugs in the callers, then > this patch makes it harder to find those bugs. >=20 > I'm actually 60:40 in favor of this patch (I think the convenience > outweighs an audit of fixing all callers); but if we do that, then we > might also want to intentionally switch existing callers to pass > negative values rather than declaring that passing a negative value is a > bug. --=20 Amos. --ibTvN161/egqYuK8 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUWv2/AAoJELxSv6I5vP9jgkMP/j2Wk87vU0VKcjEcmOFsA523 M64C/93dGCPX7gjpf3ShZQCA6YZ6qII+0MhwdZy+GsfxE4/XcSq8qGt6r+aa+tG+ QLTPdXpF2pzxTNflhev6zUn9GjLIU+ae3rv4r93t+DWmZoeZrGacNoZefWy1Hxur MV2oiHnCbW5GN0BDtpdxt1yQy3TUVBtdyt5kLTG/8nNsGCWgyBZmCvIUsJ19079G YhesMVcQkScN2Lf2/x5GW4cuQhvyQccn+odtRjhx222V0NKE+owV/3HbdOzk93tf 9l998ezz3MbvI7N4Td6/TgtFWipbfUdneOwIltZjYMb3IVVgC5irCOd7xp23NYQy 1fFQxkZ6sMMHFqxMzQJ3guZ+XFEmiDzKwTLXf59tX7lad8A0D71o+8zAR0vzbqIy cCLxYfwS6AoQ02N8X+t0rCrFU/7k4ifj1z993u8MdIrCkVMfTL1cJcgjbjN4nAdL 5liiBJJg3x7SzbkG0Sg76071N5WE7gWEPBSasVcBDh0bi/qs9enAmHzi3g1m+7Sq fq9pilMxqPbOxvqnS/xGbQOqbCgc8fpF1sPPyyDvWh5pjhQQ08PAL5JjmuOWVtpJ IxnNIAsnfJvjp+RYkt65Nt90KERjJu0t8d7qCnHnyLzCHXshdFAxKSNE6Hyc4s4e PXiQqTyTjhBVdMPWFKzf =xFdl -----END PGP SIGNATURE----- --ibTvN161/egqYuK8--