Re: [PATCH v2 04/21] hw/block: Pass DeviceState to pflash_cfi01_get_memory()

[Peter Maydell]

On Mon, 9 Jan 2023 at 23:43, Bernhard Beschow <shentey@gmail.com> wrote:
>
>
>
> Am 9. Januar 2023 12:08:16 UTC schrieb "Philippe Mathieu-Daudé" 
> <philmd@linaro.org>:
> >The point of a getter() function is to not expose the structure
> >internal fields. Otherwise callers could simply access the
> >PFlashCFI01::mem field.

"modern" QOM style somewhat relies on providing a struct definition
for a device and trusting the code that uses the device not to
peek into its private fields. In this case we're only passing
a pointer anyway, so as you say we can just use a typedef if we want.

> The getter also works with a typedef which doesn't need the structure exposed.
>
> >Have the callers pass a DeviceState* argument. The QOM
> >type check is done in the callee.
>
> Performing the cast inside the getter is essentially "lying" about the 
> getter's real interface which requires a PFlashCFI01 type to work properly. 
> Weakening the typing to the super type also weakens the compiler's ability to 
> catch mistakes at compile time. These mistakes can now only be found by 
> actually running the code or by analyzing the code very, very carefully.

Yes, I'm not super-opposed to these patches but I did find
myself wondering whether adding all these casts in the caller
is really an improvement and what we're trying to achieve.

thanks
-- PMM