All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Thomas Huth" <thuth@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	qemu-devel@nongnu.org, fam@euphon.net, kwolf@redhat.com,
	hreitz@redhat.com, groug@kaod.org, qemu_oss@crudebyte.com,
	Alistair.Francis@wdc.com, bin.meng@windriver.com,
	palmer@dabbelt.com, marcandre.lureau@redhat.com,
	pbonzini@redhat.com, yuval.shaia.ml@gmail.com,
	marcel.apfelbaum@gmail.com, mst@redhat.com, quintela@redhat.com,
	dgilbert@redhat.com, pavel.dovgaluk@ispras.ru,
	alex.bennee@linaro.org, peterx@redhat.com, david@redhat.com,
	mrolnik@gmail.com, gaosong@loongson.cn, yangxiaojuan@loongson.cn,
	aurelien@aurel32.net, jiaxun.yang@flygoat.com,
	aleksandar.rikalo@syrmia.com, jcmvbkbc@gmail.com,
	berrange@redhat.com, lvivier@redhat.com,
	suhang16@mails.ucas.ac.cn, chen.zhang@intel.com,
	lizhijian@fujitsu.com, stefanha@redhat.com,
	qemu-block@nongnu.org, qemu-riscv@nongnu.org,
	qemu-ppc@nongnu.org, virtio-fs@redhat.com
Subject: Re: [PATCH] cleanup: Tweak and re-run return_directly.cocci
Date: Tue, 22 Nov 2022 14:26:54 +0100	[thread overview]
Message-ID: <875yf7ce75.fsf@pond.sub.org> (raw)
In-Reply-To: <CAFEAcA-8tyYDMaZgYvhrG5G001HzgkCUoTUMbChDteJ+q-r8yA@mail.gmail.com> (Peter Maydell's message of "Tue, 22 Nov 2022 12:44:37 +0000")

Peter Maydell <peter.maydell@linaro.org> writes:

> On Tue, 22 Nov 2022 at 08:58, Markus Armbruster <armbru@redhat.com> wrote:
>> I don't think complete detailed review is necessary or even sensible.
>>
>> Review should start with the Coccinelle script:
>>
>>     // replace 'R = X; return R;' with 'return X;'
>>     @@
>>     identifier VAR;
>>     expression E;
>>     type T;
>>     identifier F;
>>     @@
>>      T F(...)
>>      {
>>          ...
>>     -    T VAR;
>>          ... when != VAR
>>
>>     -    VAR = (E);
>>     -    return VAR;
>>     +    return E;
>>          ... when != VAR
>>      }
>>
>> What could go wrong?  Not a rhetorical question!
>
> The obvious answer is "you might have got your manual tweaking
> wrong". A purely mechanised patch I can review by looking at
> the script and maybe eyeballing a few instances of the change;
> a change that is 99% mechanised and 1% hand-written I need to
> run through to find the hand-written parts.

Define "handwritten" :)

If reverting unwanted line-breaks and blank lines counts, then I can
make two patches, one straight from Coccinelle, and one that reverts the
unwanted crap.  The first one will be larger and more annoying to review
than this one.  A clear loss in my book, but I'm the patch submitter,
not a patch reviewer, so my book doesn't matter.

Else, we're down to one file, which I already offered to split off.

> But mostly this patch is hard to review for its sheer size,
> mechanical changes or not. A 3000 line patchmail is so big that
> the UI on my mail client gets pretty unwieldy.

With the manual one split off, target/xtensa/ dropped as requested by
Max, and tests/tcg/mips/ dropped because its status is unclear (and I
start to find it hard to care), we're down to

 28 files changed, 81 insertions(+), 221 deletions(-)

This will be v2.



WARNING: multiple messages have this Message-ID (diff)
From: Markus Armbruster <armbru@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Thomas Huth" <thuth@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	qemu-devel@nongnu.org, fam@euphon.net, kwolf@redhat.com,
	hreitz@redhat.com, groug@kaod.org, qemu_oss@crudebyte.com,
	Alistair.Francis@wdc.com, bin.meng@windriver.com,
	palmer@dabbelt.com, marcandre.lureau@redhat.com,
	pbonzini@redhat.com, yuval.shaia.ml@gmail.com,
	marcel.apfelbaum@gmail.com, mst@redhat.com, quintela@redhat.com,
	dgilbert@redhat.com, pavel.dovgaluk@ispras.ru,
	alex.bennee@linaro.org, peterx@redhat.com, david@redhat.com,
	mrolnik@gmail.com, gaosong@loongson.cn, yangxiaojuan@loongson.cn,
	aurelien@aurel32.net, jiaxun.yang@flygoat.com,
	aleksandar.rikalo@syrmia.com, jcmvbkbc@gmail.com,
	berrange@redhat.com, lvivier@redhat.com,
	suhang16@mails.ucas.ac.cn, chen.zhang@intel.com,
	lizhijian@fujitsu.com, stefanha@redhat.com,
	qemu-block@nongnu.org, qemu-riscv@nongnu.org,
	qemu-ppc@nongnu.org, virtio-fs@redhat.com
Subject: Re: [Virtio-fs] [PATCH] cleanup: Tweak and re-run return_directly.cocci
Date: Tue, 22 Nov 2022 14:26:54 +0100	[thread overview]
Message-ID: <875yf7ce75.fsf@pond.sub.org> (raw)
In-Reply-To: <CAFEAcA-8tyYDMaZgYvhrG5G001HzgkCUoTUMbChDteJ+q-r8yA@mail.gmail.com> (Peter Maydell's message of "Tue, 22 Nov 2022 12:44:37 +0000")

Peter Maydell <peter.maydell@linaro.org> writes:

> On Tue, 22 Nov 2022 at 08:58, Markus Armbruster <armbru@redhat.com> wrote:
>> I don't think complete detailed review is necessary or even sensible.
>>
>> Review should start with the Coccinelle script:
>>
>>     // replace 'R = X; return R;' with 'return X;'
>>     @@
>>     identifier VAR;
>>     expression E;
>>     type T;
>>     identifier F;
>>     @@
>>      T F(...)
>>      {
>>          ...
>>     -    T VAR;
>>          ... when != VAR
>>
>>     -    VAR = (E);
>>     -    return VAR;
>>     +    return E;
>>          ... when != VAR
>>      }
>>
>> What could go wrong?  Not a rhetorical question!
>
> The obvious answer is "you might have got your manual tweaking
> wrong". A purely mechanised patch I can review by looking at
> the script and maybe eyeballing a few instances of the change;
> a change that is 99% mechanised and 1% hand-written I need to
> run through to find the hand-written parts.

Define "handwritten" :)

If reverting unwanted line-breaks and blank lines counts, then I can
make two patches, one straight from Coccinelle, and one that reverts the
unwanted crap.  The first one will be larger and more annoying to review
than this one.  A clear loss in my book, but I'm the patch submitter,
not a patch reviewer, so my book doesn't matter.

Else, we're down to one file, which I already offered to split off.

> But mostly this patch is hard to review for its sheer size,
> mechanical changes or not. A 3000 line patchmail is so big that
> the UI on my mail client gets pretty unwieldy.

With the manual one split off, target/xtensa/ dropped as requested by
Max, and tests/tcg/mips/ dropped because its status is unclear (and I
start to find it hard to care), we're down to

 28 files changed, 81 insertions(+), 221 deletions(-)

This will be v2.

  reply	other threads:[~2022-11-22 13:27 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-21 14:01 [PATCH] cleanup: Tweak and re-run return_directly.cocci Markus Armbruster
2022-11-21 14:01 ` [Virtio-fs] " Markus Armbruster
2022-11-21 14:36 ` Peter Maydell
2022-11-21 14:36   ` [Virtio-fs] " Peter Maydell
2022-11-21 15:57   ` Markus Armbruster
2022-11-21 15:57     ` [Virtio-fs] " Markus Armbruster
2022-11-21 16:03   ` Philippe Mathieu-Daudé
2022-11-21 16:03     ` [Virtio-fs] " Philippe Mathieu-Daudé
2022-11-21 16:32     ` Markus Armbruster
2022-11-21 16:32       ` [Virtio-fs] " Markus Armbruster
2022-11-21 16:34       ` Thomas Huth
2022-11-21 16:34         ` [Virtio-fs] " Thomas Huth
2022-11-22  8:58         ` Markus Armbruster
2022-11-22  8:58           ` [Virtio-fs] " Markus Armbruster
2022-11-22 12:44           ` Peter Maydell
2022-11-22 12:44             ` [Virtio-fs] " Peter Maydell
2022-11-22 13:26             ` Markus Armbruster [this message]
2022-11-22 13:26               ` Markus Armbruster
2022-11-22 13:45               ` Peter Maydell
2022-11-22 13:45                 ` [Virtio-fs] " Peter Maydell
2022-11-22 14:51           ` Philippe Mathieu-Daudé
2022-11-22 14:51             ` [Virtio-fs] " Philippe Mathieu-Daudé
2022-11-21 16:42 ` Max Filippov
2022-11-21 16:42   ` [Virtio-fs] " Max Filippov
2022-11-21 17:00   ` Markus Armbruster
2022-11-21 17:00     ` [Virtio-fs] " Markus Armbruster
2022-11-22 15:03   ` Philippe Mathieu-Daudé
2022-11-22 15:03     ` [Virtio-fs] " Philippe Mathieu-Daudé
2022-11-22 15:12     ` Peter Maydell
2022-11-22 15:12       ` [Virtio-fs] " Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=875yf7ce75.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=Alistair.Francis@wdc.com \
    --cc=aleksandar.rikalo@syrmia.com \
    --cc=alex.bennee@linaro.org \
    --cc=aurelien@aurel32.net \
    --cc=berrange@redhat.com \
    --cc=bin.meng@windriver.com \
    --cc=chen.zhang@intel.com \
    --cc=david@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=fam@euphon.net \
    --cc=gaosong@loongson.cn \
    --cc=groug@kaod.org \
    --cc=hreitz@redhat.com \
    --cc=jcmvbkbc@gmail.com \
    --cc=jiaxun.yang@flygoat.com \
    --cc=kwolf@redhat.com \
    --cc=lizhijian@fujitsu.com \
    --cc=lvivier@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=mrolnik@gmail.com \
    --cc=mst@redhat.com \
    --cc=palmer@dabbelt.com \
    --cc=pavel.dovgaluk@ispras.ru \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=peterx@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=qemu_oss@crudebyte.com \
    --cc=quintela@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=suhang16@mails.ucas.ac.cn \
    --cc=thuth@redhat.com \
    --cc=virtio-fs@redhat.com \
    --cc=yangxiaojuan@loongson.cn \
    --cc=yuval.shaia.ml@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.