From: Anand J <anand.indukala@gmail.com>
To: Eric Blake <eblake@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-trivial@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] clean-up: removed duplicate #includes
Date: Sun, 9 Oct 2016 15:03:25 +0530 [thread overview]
Message-ID: <CANDALvmjgU4JERC1e7DaUzzw8C1LonikfpZrwo9DpbiEBdaOxg@mail.gmail.com> (raw)
In-Reply-To: <45036bfe-789a-3f56-4b24-a507b9e9140a@redhat.com>
On Sun, Oct 9, 2016 at 3:04 AM, Eric Blake <eblake@redhat.com> wrote:
> On 10/08/2016 06:32 AM, Anand J wrote:
> > I have replied for the questions inline. Also I'll make changes to the
> > patch as per the review and send the updated one.
> >
> > Thanks,
> > Anand
> >
> > On Fri, Oct 7, 2016 at 8:01 PM, Eric Blake <eblake@redhat.com> wrote:
> >
> >> On 10/07/2016 03:46 AM, Anand J wrote:
> >>> Some files contain multiple #includes of the same header file.
> >>> Removed most of those unnecessary duplicate entries.
> >>
> >> How did you find these? Is it a repeatable formula for rerunning a year
> >> from now to find new culprits? If so, listing it in the commit message
> >> would be worthwhile. Is it something we should add to
> >> scripts/clean-includes?
> >>
> > I executed the following bash script to get multiple occurrences of
> > #includes and manually checked each and every file from the output.
> >
> > grep -r --exclude-dir=bin/ "#include" | sort | uniq -c | awk '{if ($1 >
> 1)
> > print $2}'
>
> Seems simple enough that we ought to make it part of the
> scripts/clean-includes, rather than remembering to manually run it.
>
scripts/clean-includes does an in place modification of the duplicate
entries in the files.
There is no guarantee that my script with return all genuine duplicate
entries. Some can be false positive as well.
Are you asking me to add this script in as a comment in
scripts/clean-includes files? Like the one
for checking if all the files start with #include "osdep.h" ?
# for i in $(git ls-tree --name-only HEAD) ; do test -f $i && \
# grep -E '^# *include' $i | head -1 | grep 'osdep.h' ; test $? != 0 && \
# echo $i ; done
> >
> > But there are lot of noise in the output of this command. Most of the
> > multiple #includes were not genuine issue. @Eric Do you want me to add
> this
> > in the comment?
>
> Yes - whatever we end up with (whether a manual script or a run of
> scripts/clean-includes), documenting the process used makes it easier to
> verify that you didn't miss anything, and for someone else to repeat the
> process in another year when some mistakes have crept back in during the
> meantime.
>
agree. Adding to the comments for future reference.
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
>
next prev parent reply other threads:[~2016-10-09 9:33 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-07 8:46 [Qemu-devel] [PATCH] clean-up: removed duplicate #includes Anand J
2016-10-07 14:31 ` Eric Blake
2016-10-07 14:49 ` Peter Maydell
2016-10-08 11:32 ` Anand J
2016-10-08 21:34 ` Eric Blake
2016-10-09 9:33 ` Anand J [this message]
2016-10-10 16:25 ` Eric Blake
2016-10-07 14:48 ` 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=CANDALvmjgU4JERC1e7DaUzzw8C1LonikfpZrwo9DpbiEBdaOxg@mail.gmail.com \
--to=anand.indukala@gmail.com \
--cc=eblake@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-trivial@nongnu.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).