Re: [RFC PATCH 1/2] cpus: Introduce const_cpu_env() and const_env_archcpu()

[Daniel Henrique Barboza]

On 1/16/25 8:04 PM, Philippe Mathieu-Daudé wrote:
> const_cpu_env() is similar to cpu_env() but return a const
> CPU 'env' state.
> Same for const_env_archcpu() w.r.t. env_archcpu().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/cpu-common.h | 5 +++++
>   include/hw/core/cpu.h     | 6 ++++++
>   2 files changed, 11 insertions(+)
>
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index b1d76d69850..f765e97a973 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -250,6 +250,11 @@ static inline ArchCPU *env_archcpu(CPUArchState *env)
>       return (void *)env - sizeof(CPUState);
>   }
>   
> +static inline const ArchCPU *const_env_archcpu(const CPUArchState *env)
> +{
> +    return (const void *)env - sizeof(CPUState);
> +}
> +

Can't we get away with something like:
 
+static inline const ArchCPU *const_env_archcpu(CPUArchState *env)
+{
+    return env_archcpu(env);
+}
+
 

We're just adding a 'const' to a pointer that env_archcpu() already retrieves,
so might as well use env_archcpu(). The 'const' will be added by the compiler.
This code builds without warnings in gcc and clang in my env (Fedora 41).

If we want to be a bit more explicit I suggest:

+static inline const ArchCPU *const_env_archcpu(CPUArchState *env)
+{
+    return (const ArchCPU *)env_archcpu(env);
+}
+

Same observation with const_cpu_env() and cpu_env(). Thanks,


Daniel

>   /**
>    * env_cpu_const(env)
>    * @env: The architecture environment
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index c3ca0babcb3..ecb31221b26 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -588,6 +588,12 @@ static inline CPUArchState *cpu_env(CPUState *cpu)
>       return (CPUArchState *)(cpu + 1);
>   }
>   
> +static inline const CPUArchState *const_cpu_env(const CPUState *cpu)
> +{
> +    /* We validate that CPUArchState follows CPUState in cpu-all.h. */
> +    return (const CPUArchState *)(cpu + 1);
> +}
> +
>   typedef QTAILQ_HEAD(CPUTailQ, CPUState) CPUTailQ;
>   extern CPUTailQ cpus_queue;
>