Re: [PATCH v16 78/99] target/arm: cpu-exceptions, cpu-exceptions-aa64: new modules

[Claudio Fontana]

On 6/5/21 10:50 PM, Richard Henderson wrote:
> On 6/4/21 8:52 AM, Alex Bennée wrote:
>> diff --git a/target/arm/cpu-exceptions-aa64.h 
>> b/target/arm/cpu-exceptions-aa64.h
>> new file mode 100644
>> index 0000000000..64f800a15d
>> --- /dev/null
>> +++ b/target/arm/cpu-exceptions-aa64.h
>> @@ -0,0 +1,28 @@
>> +/*
>> + * QEMU AArch64 CPU Exceptions Sysemu code
>> + *
>> + * Copyright (c) 2012 SUSE LINUX Products GmbH
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version 2
>> + * of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, see
>> + *<http://www.gnu.org/licenses/gpl-2.0.html>
>> + */
>> +
>> +#ifndef CPU_EXCEPTIONS_AA64_H
>> +#define CPU_EXCEPTIONS_AA64_H
>> +
>> +#include "cpu.h"
>> +
>> +void arm_cpu_do_interrupt_aarch64(CPUState *cs);
>> +
>> +#endif /* CPU_EXCEPTIONS_AA64_H */
> 
> I don't see the point in the excessive replication of header files, for 
> exactly 
> one declaration.  This is not the first example.
> 
> What's wrong with internal.h? 

Doesn't mean anything and leads to a mess of unrelated code.
I think we should be better at creating meaningful modules, instead of 
collections of unrelated functions.

Not defending this particular change, just answering your specific question.

Thanks,

C


> Or some other header that collects sysemu 
> specific declarations?
> 
> Also, "cpu.h" is not required by this declaration, as I've pointed out before.
> 
> 
> r~
>