From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43829) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bBTXC-0008Ik-7i for qemu-devel@nongnu.org; Fri, 10 Jun 2016 17:00:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bBTX9-0004LN-S8 for qemu-devel@nongnu.org; Fri, 10 Jun 2016 17:00:05 -0400 References: <1465589538-24998-1-git-send-email-ehabkost@redhat.com> <1465589538-24998-3-git-send-email-ehabkost@redhat.com> From: Eric Blake Message-ID: <575B2A4B.8090705@redhat.com> Date: Fri, 10 Jun 2016 14:59:55 -0600 MIME-Version: 1.0 In-Reply-To: <1465589538-24998-3-git-send-email-ehabkost@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="mRDtCbxSpkoBJJBkGmK5d3CgOoSfPomFo" Subject: Re: [Qemu-devel] [PATCH v2 2/3] error: Remove unnecessary local_err variables List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , qemu-devel@nongnu.org, Markus Armbruster Cc: kwolf@redhat.com, borntraeger@de.ibm.com, qemu-block@nongnu.org, cornelia.huck@de.ibm.com, mreitz@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --mRDtCbxSpkoBJJBkGmK5d3CgOoSfPomFo Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 06/10/2016 02:12 PM, Eduardo Habkost wrote: > This patch simplifies code that uses a local_err variable just to > immediately use it for an error_propagate() call. >=20 > Coccinelle patch used to perform the changes added to > scripts/coccinelle/remove_local_err.cocci. >=20 > Signed-off-by: Eduardo Habkost > --- > block.c | 8 ++------ > block/raw-posix.c | 8 ++------ > block/raw_bsd.c | 4 +--- > blockdev.c | 16 +++++----------- > hw/s390x/s390-virtio-ccw.c | 5 +---- > hw/s390x/virtio-ccw.c | 28 +++++++----------------= ----- > scripts/coccinelle/remove_local_err.cocci | 27 +++++++++++++++++++++++= ++++ > target-i386/cpu.c | 4 +--- > 8 files changed, 46 insertions(+), 54 deletions(-) > create mode 100644 scripts/coccinelle/remove_local_err.cocci >=20 > +++ b/block.c > @@ -294,14 +294,12 @@ typedef struct CreateCo { > =20 > static void coroutine_fn bdrv_create_co_entry(void *opaque) > { > - Error *local_err =3D NULL; > int ret; > =20 > CreateCo *cco =3D opaque; > assert(cco->drv); > =20 > - ret =3D cco->drv->bdrv_create(cco->filename, cco->opts, &local_err= ); > - error_propagate(&cco->err, local_err); > + ret =3D cco->drv->bdrv_create(cco->filename, cco->opts, &cco->err)= ; > cco->ret =3D ret; This hunk doesn't get simplified by 3/3; you may want to consider a manual followup to drop 'int ret' and just assign cco->drv->bdrv_create() directly to cco->ret. But doesn't change this patch. > +++ b/blockdev.c > @@ -3654,7 +3654,6 @@ void qmp_blockdev_mirror(const char *device, cons= t char *target, > BlockBackend *blk; > BlockDriverState *target_bs; > AioContext *aio_context; > - Error *local_err =3D NULL; > =20 > blk =3D blk_by_name(device); > if (!blk) { > @@ -3678,16 +3677,11 @@ void qmp_blockdev_mirror(const char *device, co= nst char *target, > =20 > bdrv_set_aio_context(target_bs, aio_context); > =20 > - blockdev_mirror_common(bs, target_bs, > - has_replaces, replaces, sync, > - has_speed, speed, > - has_granularity, granularity, > - has_buf_size, buf_size, > - has_on_source_error, on_source_error, > - has_on_target_error, on_target_error, > - true, true, > - &local_err); > - error_propagate(errp, local_err); > + blockdev_mirror_common(bs, target_bs, has_replaces, replaces, sync= , > + has_speed, speed, has_granularity, granular= ity, > + has_buf_size, buf_size, has_on_source_error= , > + on_source_error, has_on_target_error, > + on_target_error, true, true, errp); Coccinelle messes a bit with the formatting (the old way explicitly tried to pair related has_foo with foo). But I'm going to mess with it again with my qapi patches for passing a boxed parameter rather than lots of arguments, so don't worry about it. > +++ b/scripts/coccinelle/remove_local_err.cocci > @@ -0,0 +1,27 @@ > +// Replace unnecessary usage of local_err variable with > +// direct usage of errp argument > + > +@@ > +expression list ARGS; > +expression F2; > +identifier LOCAL_ERR; > +expression ERRP; > +idexpression V; > +typedef Error; > +expression I; > +@@ > + { > + ... > +- Error *LOCAL_ERR; > + ... when !=3D LOCAL_ERR > +( > +- F2(ARGS, &LOCAL_ERR); > +- error_propagate(ERRP, LOCAL_ERR); > ++ F2(ARGS, ERRP); > +| > +- V =3D F2(ARGS, &LOCAL_ERR); > +- error_propagate(ERRP, LOCAL_ERR); > ++ V =3D F2(ARGS, ERRP); > +) > + ... when !=3D LOCAL_ERR > + } Looks good. Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --mRDtCbxSpkoBJJBkGmK5d3CgOoSfPomFo 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/ iQEcBAEBCAAGBQJXWypLAAoJEKeha0olJ0NqD0EH/0pdWvjqcT55GOl6Q03hSMga sc7AMh/F8lqrmbNI74A9+m3Mnshi8q9w4jOFnteC1EnWOrXezkHnACTOuH0L45uc 1oh02iPYNVDMFW/d22XMGXguCE8QkUsUxz21kgEJnZz+OGVZXVmZztUguD3VRDgQ 8ls5oC4DvFUzIkZV8fIFW4LJtVqB+V5zqE29g39AGwJ2dOGyTO72iYTGZfkYHFnM 4xTpUn7/Ng15FED5ajzd7d2nN4NobVM11Z1W1/71SQdYXplqvHXPjA6V2cKiOxHm oUac7LW4RdGZtdBRjz/RnSogRlZWLO62w9b1Y+wWBuv+s6MA5fhgfPd50O4lVyc= =VqqO -----END PGP SIGNATURE----- --mRDtCbxSpkoBJJBkGmK5d3CgOoSfPomFo--