On 7/6/21 11:06 AM, Thomas Huth wrote: > On 06/07/2021 10.47, Al Cho wrote: >> On Mon, 2021-07-05 at 08:25 +0200, Claudio Fontana wrote: >>> On 7/2/21 9:25 AM, Al Cho wrote: >>>> On Thu, 2021-07-01 at 14:35 +0200, Thomas Huth wrote: >>>>> On 29/06/2021 16.19, Cho, Yu-Chen wrote: >>>>>> Splitting this functionality also allows us to make helper.c >>>>>> sysemu- >>>>>> only. >>>>>> >>>>>> Signed-off-by: Claudio Fontana <cfont...@suse.de> >>>>>> Signed-off-by: Cho, Yu-Chen <a...@suse.com> >>>>>> Acked-by: Cornelia Huck <coh...@redhat.com> >>>>>> --- >>>>>> target/s390x/cpu-dump.c | 176 >>>>>> +++++++++++++++++++++++++++++++++++++++ >>>>> >>>>> Apart from the dump() function, the other functions here are are >>>>> used >>>>> in >>>>> other contexts, too, so maybe the name is not very appropriate >>>>> here... >>>>> What >>>>> about naming it "cpu-state.c" instead? Or include the functions >>>>> in >>>>> cpu.c >>>>> directly? >>>>> >>>> >>>> ok, I think naming it "cpu-state.c" would make more sense. >>>> >>>> Thanks, >>>> AL >>>> >>> >>> For context, cpu-dump.c mimics how this is done on x86, >>> >>> so rather than coming up with creative new names for each >>> architecture, >> >> I think Claudio is right, I didn't recognize it before. sorry. >> >>> I'd rather either put the code into cpu.c, or just keep the existing >>> "cpu-dump.c" as in the initially proposed patch, which looks like the >>> best option to me. >>> >> >> For me just keep the existing "cpu-dump.c" as in the initially proposed >> patch would be the better one option. >> But it's also good to me if we keep the dump() function in cpu-dump.c >> and put other functions into cpu.c. > > FWIW, if you don't like cpu-state.c, I'd vote for putting the dump() > function into cpu-dump.c and put the other functions into cpu.c instead. > > Thomas >
Ah I see the issue now, the patch currently includes functions in cpu-dump.c that are not really cpu dump functions, and should go back into cpu.c . Agreed, this seems to be the next step. Thanks, Claudio
