From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41639) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fO0dO-0002qe-PL for qemu-devel@nongnu.org; Wed, 30 May 2018 08:55:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fO0dN-0005RI-NH for qemu-devel@nongnu.org; Wed, 30 May 2018 08:55:22 -0400 Date: Wed, 30 May 2018 15:55:12 +0300 From: "Michael S. Tsirkin" Message-ID: <20180530155301-mutt-send-email-mst@kernel.org> References: <20180528232719.4721-1-f4bug@amsat.org> <20180528232719.4721-10-f4bug@amsat.org> <20180529134053.1ea9f46d.cohuck@redhat.com> <11289f38-6d0f-74c9-1719-83b2a3ed2d97@amsat.org> <20180530074026-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 09/21] target: Do not include "exec/exec-all.h" if it is not necessary List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Philippe =?iso-8859-1?Q?Mathieu-Daud=E9?= Cc: Cornelia Huck , Thomas Huth , "qemu-devel@nongnu.org Developers" , QEMU Trivial , Paolo Bonzini , Richard Henderson , Eduardo Habkost , Marcel Apfelbaum , Peter Maydell , "Edgar E. Iglesias" , Michael Walle , Laurent Vivier , Anthony Green , Stafford Horne , David Gibson , Alexander Graf , David Hildenbrand , Max Filippov , "open list:ARM" , "open list:PowerPC" , "open list:S390" On Wed, May 30, 2018 at 03:19:19AM -0300, Philippe Mathieu-Daud=E9 wrote: > Le mer. 30 mai 2018 02:50, Philippe Mathieu-Daud=E9 a= =E9crit=A0: >=20 > On Wed, May 30, 2018 at 1:42 AM, Michael S. Tsirkin wrote: > > On Wed, May 30, 2018 at 12:12:55AM -0300, Philippe Mathieu-Daud=E9= wrote: > >> Hi Cornelia, > >> > >> On 05/29/2018 08:40 AM, Cornelia Huck wrote: > >> > On Mon, 28 May 2018 20:27:07 -0300 > >> > Philippe Mathieu-Daud=E9 wrote: > >> > > >> >> Code change produced with: > >> >>=A0 =A0 =A0$ git grep '#include "exec/exec-all.h"' | \ > >> >>=A0 =A0 =A0 =A0cut -d: -f-1 | \ > >> >>=A0 =A0 =A0 =A0xargs egrep -L "(cpu_address_space_init|cpu_loo= p_|tlb_|tb_| > GETPC|singlestep|TranslationBlock)" | \ > >> > > >> > Hm, does this expression catch all files that need to include > >> > exec-all.h? The resulting patch seems fine, though. > >> > >> No, not all :/ > >> I started with "(cpu_loop_|tlb_|tb_)" then kept brutebuilding un= til no > >> more errors appear. In 2 more steps I added "cpu_address_space_i= nit|" > >> then "|GETPC|singlestep|TranslationBlock". Quick and dirty enoug= h for my > >> goal than trying to build a regex to explode function/struct nam= es from > >> headers. This is a clever way to do it for long term command reu= se taken > >> from commit messages... > > > > Brutebuilding isn't a good way to find unused includes, some othe= r header > > might pull in an include you are trying to remove for its own pur= poses. > > If you want to try brutebuilding you must also verify that's > > not the case - e.g. look at the dependency file generated. >=20 >=20 > I think I get it for headers, but here I'm only modifying .c source fil= es.=A0 Same thing - if you call a function from a header, you should include that header even if some other header happens to pull it in. Otherwise someone might cleanup that other header and suddenly your build breaks. > I'll restrict the shell command to .c=A0 I don't think it matters. >=20 >=20 > Hmm you mean the .d files in the build dir? >=20 > i.e. > $ find lm32-softmmu -name \*.d -exec fgrep -L exec-all.h {} \; | se= d > -e 's/.*-softmmu\/\(.*\)d$/\1c/' > dump.c > numa.c > tcg/tcg-common.c > tcg/optimize.c > tcg/tcg-op-gvec.c > tcg/tcg-op-vec.c > accel/tcg/tcg-runtime-gvec.c > accel/tcg/tcg-all.c > accel/accel.c > accel/stubs/whpx-stub.c > accel/stubs/hvf-stub.c > accel/stubs/kvm-stub.c > accel/stubs/hax-stub.c > hw/lm32/milkymist.c > hw/lm32/lm32_boards.c > hw/core/generic-loader.c > ... >=20 > Then look why each .c included it? >=20 > > > >> >>=A0 =A0 =A0 =A0xargs sed -i.bak '/#include "exec\/exec-all.h"/= d' > >> >> > >> >> Signed-off-by: Philippe Mathieu-Daud=E9 >=20