Re: [Qemu-devel] [PATCH] clean-up: removed duplicate #includes

[Anand J]

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 <address@hidden> 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}'

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?



>
> >
> > Signed-off-by: Anand J <address@hidden>
> > ---
>
> > +++ b/disas/libvixl/vixl/globals.h
> > @@ -46,7 +46,6 @@
> >  #include <assert.h>
> >  #include <stdarg.h>
> >  #include <stdio.h>
> > -#include <stdint.h>
> >  #include <stdlib.h>
> >  #include <stddef.h>
> >  #include "vixl/platform.h"
>
> scripts/clean-includes intentionally ignores disas/libvixl because that
> source is copied from elsewhere with minimal changes; are you sure this
> hunk is appropriate?
>

stdint.h is already included as the first #include in this file. But since
this is a thrid-party file as per Peter, I'll remove this change.


> > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > @@ -13,7 +13,6 @@
> >  #include "qemu/osdep.h"
> >  #include "qapi/error.h"
> >  #include "hw/pci/pci.h"
> > -#include "hw/pci/pci_bus.h"
> >  #include "hw/pci/pci_host.h"
> >  #include "hw/pci/pci_bus.h"
>
> Changes like this are obviously correct...
>
> >  #include "hw/pci/pci_bridge.h"
> > diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> > index 4b2f07a..d01798f 100644
> > --- a/hw/ppc/ppc405_boards.c
> > +++ b/hw/ppc/ppc405_boards.c
> > @@ -37,7 +37,6 @@
> >  #include "qemu/log.h"
> >  #include "qemu/error-report.h"
> >  #include "hw/loader.h"
> > -#include "sysemu/block-backend.h"
> >  #include "sysemu/blockdev.h"
> >  #include "exec/address-spaces.h"
>
> ...while changes like this require looking at context. But the nice part
> of this patch is that if it compiles, it is correct...
>
> > +++ b/hw/usb/dev-mtp.c
> > @@ -17,7 +17,6 @@
> >  #include <sys/statvfs.h>
> >  #ifdef CONFIG_INOTIFY1
> >  #include <sys/inotify.h>
> > -#include "qapi/error.h"
> >  #include "qemu/main-loop.h"
> >  #endif
>
> ...well, ones like this are a little trickier (if CONFIG_INOTIFY1 is not
> defined, a completed compilation is no indication of success - but
> reading context shows it is correct, and the duplicate include was just
> outside of the diff context).
>
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
>