On 23 March 2018 at 16:51, Laurent Vivier <laur...@vivier.eu> wrote: > Le 23/03/2018 à 15:22, Peter Maydell a écrit : >> On 22 March 2018 at 21:58, Laurent Vivier <laur...@vivier.eu> wrote: >>> move all target specific parts to >>> signal.inc.c in arch directory >>> >>> Signed-off-by: Laurent Vivier <laur...@vivier.eu> >>> --- >>> linux-user/aarch64/signal.inc.c | 556 +++ >>> linux-user/alpha/signal.inc.c | 258 ++ >>> linux-user/arm/signal.inc.c | 749 +++++ >>> linux-user/cris/signal.inc.c | 166 + >>> linux-user/hppa/signal.inc.c | 188 ++ >>> linux-user/i386/signal.inc.c | 579 ++++ >>> linux-user/m68k/signal.inc.c | 406 +++ >>> linux-user/microblaze/signal.inc.c | 226 ++ >>> linux-user/mips/signal.inc.c | 378 +++ >>> linux-user/mips64/signal.inc.c | 1 + >>> linux-user/nios2/signal.inc.c | 232 ++ >>> linux-user/openrisc/signal.inc.c | 209 ++ >>> linux-user/ppc/signal.inc.c | 667 ++++ >>> linux-user/riscv/signal.inc.c | 196 ++ >>> linux-user/s390x/signal.inc.c | 305 ++ >>> linux-user/sh4/signal.inc.c | 327 ++ >>> linux-user/signal.c | 6487 >>> +----------------------------------- >>> linux-user/sparc/signal.inc.c | 601 ++++ >>> linux-user/sparc64/signal.inc.c | 1 + >>> linux-user/tilegx/signal.inc.c | 163 + >>> linux-user/x86_64/signal.inc.c | 1 + >>> linux-user/xtensa/signal.inc.c | 253 ++ >>> 22 files changed, 6463 insertions(+), 6486 deletions(-) >> >> I think anything with a diffstat like this is basically >> unreviewable, at least for me :-( >> >>> diff --git a/linux-user/aarch64/signal.inc.c >>> b/linux-user/aarch64/signal.inc.c >>> new file mode 100644 >>> index 0000000000..28fa0f2f22 >>> --- /dev/null >>> +++ b/linux-user/aarch64/signal.inc.c >> >> I was hoping we could have these be standalone .c files, >> rather than just #included from linux-user/signal.c. >> I appreciate this requires a bit more teasing out of what >> needs to become non-static in the common code signal.c >> to be callable from the per-target code, though. >> > > Thank you for your review. > > I tried the easy way first... but I'm going to try to do standalone c files.
Structuring your patchset so it can move one architecture at a time out ouf the common file will probably help in making the patch more reviewable, though it's likely a bit more painful to do. I think at the time I was looking at this I was contemplating an approach like: (1) for architecture A, move that arch's code from signal.c to $ARCH/signal.c, and temporarily use a #include to include it from signal.c (2) repeat for architectures B, C, ..., one per patch (3) make the makefile and code changes needed to drop the #includes and build $ARCH/signal.c as standalone files, as a final patch and note in the commit messages for the 1, 2... patches that they're purely code movement with no textual changes. thanks -- PMM
