Re: [RFC v6 08/13] target/s390x: split cpu-dump from helper.c

[Claudio Fontana]

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 <cfontana@suse.de>
>>>>>> Signed-off-by: Cho, Yu-Chen <acho@suse.com>
>>>>>> Acked-by: Cornelia Huck <cohuck@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