[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 00/59] trivial unneeded labels cleanup

From: Daniel Henrique Barboza
Subject: Re: [PATCH v1 00/59] trivial unneeded labels cleanup
Date: Fri, 10 Jan 2020 17:31:24 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1

On 1/10/20 4:05 PM, Eric Blake wrote:
On 1/6/20 12:23 PM, Daniel Henrique Barboza wrote:

This is the type of cleanup I've contributed to Libvirt
during the last year. Figured QEMU also deserves the same

The idea here is remove unneeded labels. By 'unneeded' I
mean labels that does nothing but a 'return' call. One
common case is something like this:

if ()
     goto cleanup;
     return 0;

This code can be simplified to:

if ()
     return 0;

How much of this work is done manually, and how much via Coccinelle?

I'm still getting along with Coccinelle. I'm able to do simple matches but
couldn't make it work to match this scenario. I didn't invest too much
time on it though.

  qga/commands-win32.c          | 17 ++++---
  target/mips/mips-semi.c       | 15 +++---
  target/unicore32/softmmu.c    | 23 +++-------
  util/aio-posix.c              |  3 +-
  util/module.c                 | 11 ++---

Hmm, no change to scripts/coccinelle, so presumably all manual :(

I used pcregrep:

$ pcregrep --exclude-dir=build --exclude-dir=docs --exclude-dir=scripts -r -n -M 
':\n*\s*return' . > clean_labels

(note: there was a lot more of --exclude-dir in the original command, 
unfortunately I
didn't record it)

Then I filtered in the output file the cases where the regexp matched "switch"
labels and any other false positives like strings in comments. It's not manual
inspection, but it was crude indeed.

If we have a Coccinelle script that performs this transformation, we are more 
likely to be able to catch all places in the tree that would benefit (rather 
than relying on grep calls or other manual inspection), and more importantly, 
we can repeat the effort periodically to fix future additions that don't comply 
with the preferred style as well as backport the patch by rerunning the 
Coccinelle script with less worry of changed context.  Automated cleanups are 
always going to be easier to swallow (even if the diffstat is larger) than 
manual ad hoc cleanups.

I am not aware of how QEMU handles Coccinelle, if it is imposed via
./scripts/checkpatch.pl or something that it is ran from time to time to
suggest changes. But if it's desirable, I can see if I can cook a Coccinelle
script (or anything a bit more sophisticated than what I did)  to flag these
cases I attempted to handle with this series.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]