From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41128) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9cBj-000708-MF for qemu-devel@nongnu.org; Thu, 17 Dec 2015 12:18:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a9cBg-0006w5-3B for qemu-devel@nongnu.org; Thu, 17 Dec 2015 12:17:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36122) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9cBf-0006vF-JR for qemu-devel@nongnu.org; Thu, 17 Dec 2015 12:17:55 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 4AF84744 for ; Thu, 17 Dec 2015 17:17:55 +0000 (UTC) References: <1450371004-26866-1-git-send-email-armbru@redhat.com> <1450371004-26866-2-git-send-email-armbru@redhat.com> From: Eric Blake Message-ID: <5672EE3D.3080901@redhat.com> Date: Thu, 17 Dec 2015 10:17:49 -0700 MIME-Version: 1.0 In-Reply-To: <1450371004-26866-2-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="52p8kQlUMpccMet8QDCBegjH1mG9OuvRd" Subject: Re: [Qemu-devel] [PATCH v2 01/23] qemu-nbd: Replace BSDism by error_report() 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) --52p8kQlUMpccMet8QDCBegjH1mG9OuvRd Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 12/17/2015 09:49 AM, Markus Armbruster wrote: > Coccinelle semantic patch >=20 > @@ > expression E; > expression list ARGS; > @@ > - errx(E, ARGS); > + error_report(ARGS); > + exit(E); > @@ > expression E, FMT; > expression list ARGS; > @@ > - err(E, FMT, ARGS); > + error_report(FMT /*": %s"*/, ARGS, strerror(errno)); > + exit(E); >=20 > followed by a replace of '"/*": %s"*/' by ' : %s"'. Interesting approach to working around a coccinelle shortcoming. Hmm. Should we add error_report_errno(), to parallel error_setg_errno()? But that can be a separate patch. >=20 > Signed-off-by: Markus Armbruster > --- > qemu-nbd.c | 119 ++++++++++++++++++++++++++++++++++++++---------------= -------- > 1 file changed, 75 insertions(+), 44 deletions(-) >=20 > @@ -491,13 +498,14 @@ int main(int argc, char **argv) > BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF, > &local_err); > if (local_err) { > - errx(EXIT_FAILURE, "Failed to parse detect_zeroes mode= : %s",=20 > - error_get_pretty(local_err)); > + error_report("Failed to parse detect_zeroes mode: %s",= > + error_get_pretty(local_err)); > + exit(EXIT_FAILURE); > } > if (detect_zeroes =3D=3D BLOCKDEV_DETECT_ZEROES_OPTIONS_UN= MAP && > !(flags & BDRV_O_UNMAP)) { > - errx(EXIT_FAILURE, "setting detect-zeroes to unmap is = not allowed " > - "without setting discard operation = to unmap");=20 > + error_report("setting detect-zeroes to unmap is not al= lowed " "without setting discard operation to unmap"); You'll want to fix the line-wrap on this. > @@ -509,10 +517,12 @@ int main(int argc, char **argv) > case 'o': > dev_offset =3D strtoll (optarg, &end, 0); > if (*end) { > - errx(EXIT_FAILURE, "Invalid offset `%s'", optarg); > + error_report("Invalid offset `%s'", optarg); Worth cleaning this up to use '' instead of `' at some point in the series? (Not here, though, since this patch is best when it sticks as close to the coccinelle script as possible). > + exit(EXIT_FAILURE); > } > if (dev_offset < 0) { > - errx(EXIT_FAILURE, "Offset must be positive `%s'", opt= arg); > + error_report("Offset must be positive `%s'", optarg); Obviously, any `' cleanup to '' will have quite a few callers to adjust. > @@ -534,16 +545,19 @@ int main(int argc, char **argv) > case 'P': > partition =3D strtol(optarg, &end, 0); > if (*end) { > - errx(EXIT_FAILURE, "Invalid partition `%s'", optarg); > + error_report("Invalid partition `%s'", optarg); > + exit(EXIT_FAILURE); > } > if (partition < 1 || partition > 8) { > - errx(EXIT_FAILURE, "Invalid partition %d", partition);= > + error_report("Invalid partition %d", partition); > + exit(EXIT_FAILURE); > } > break; > case 'k': > sockpath =3D optarg; > if (sockpath[0] !=3D '/') { > - errx(EXIT_FAILURE, "socket path must be absolute\n"); > + error_report("socket path must be absolute\n"); I'm guessing later in the series will kill the trailing newline? If so, then this patch is fine (again, limiting to just coccinelle here). > + exit(EXIT_FAILURE); > } > break; > case 'd': > @@ -555,10 +569,12 @@ int main(int argc, char **argv) > case 'e': > shared =3D strtol(optarg, &end, 0); > if (*end) { > - errx(EXIT_FAILURE, "Invalid shared device number '%s'"= , optarg); > + error_report("Invalid shared device number '%s'", opta= rg); > + exit(EXIT_FAILURE); > } > if (shared < 1) { > - errx(EXIT_FAILURE, "Shared device number must be great= er than 0\n"); > + error_report("Shared device number must be greater tha= n 0\n"); > + exit(EXIT_FAILURE); And another one. Maybe mention in the commit message any future cleanups planned for style issues that are exposed by this conversion? > } > break; > case 'f': > @@ -579,21 +595,23 @@ int main(int argc, char **argv) > exit(0); > break; > case '?': > - errx(EXIT_FAILURE, "Try `%s --help' for more information."= , > - argv[0]); > + error_report("Try `%s --help' for more information.", argv= [0]); > + exit(EXIT_FAILURE); > } > } > =20 > if ((argc - optind) !=3D 1) { > - errx(EXIT_FAILURE, "Invalid number of argument.\n" > - "Try `%s --help' for more information.", > - argv[0]); > + error_report("Invalid number of argument.\n" "Try `%s --help' = for more information.", > + argv[0]); Long source line worth wrapping? Line wraps and commit message improvements seem obvious, so I'm okay with adding: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --52p8kQlUMpccMet8QDCBegjH1mG9OuvRd 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/ iQEcBAEBCAAGBQJWcu49AAoJEKeha0olJ0NqVQQIAItfhsSswEXpAUk05k4Ewcj+ cp/LsKi/lMtZKjl+rtyPWNmRWb0QNJ2t7to8wphhPoKGLu7Rn1/Zwvzt95gui8dE wHu5D68emn6UcJOEZa1Yc26QwoGjmFq0SWI/n99NiqIkkoPxTfGB2IRfnUAMd4Lc klYXQvjBob7hp0TmqL+4lSaceaNc46fuZCYe3naUzDCJDsrlLIY1p96rIto6L1+b TJO01ICW+FvUpoJC3tWGpyJnJ55WC+TNUb7aEDCEOWB5mpa11HyQVJJ62Jb71e97 eNywcXg7Elhn0tPip0YjR8N9yfB9Gli/TLfYZya1KyvBGCm7iY9C5nlKIWSF6Nw= =5lUa -----END PGP SIGNATURE----- --52p8kQlUMpccMet8QDCBegjH1mG9OuvRd--