Re: [PATCH v3 2/9] hw/{arm,ppc}: Resolve unreachable code

[Bernhard Beschow]

Am 17. Oktober 2022 20:57:06 UTC schrieb "Philippe Mathieu-Daudé" 
<philmd@linaro.org>:
>On 16/10/22 14:27, Bernhard Beschow wrote:
>> pflash_cfi01_register() always returns with a non-NULL pointer (otherwise
>> it would crash internally). Therefore, the bodies of the if-statements
>> are unreachable.
>
>This is true, pflash_cfi0X_register() use an hardcoded &error_fatal.
>
>Shouldn't it be better to pass an Error* argument?

You mean that the callers would pass &error_fatal instead, including the ones 
in this patch?

>
>From the pflash API perspective I don't see much value in
>returning a PFlashCFI0X type instead of a simple DeviceState
>(but this is another story...).

It comes in handy in the following patches when retrieving the memory region 
for memory_region_add_subregion() using pflash_cfi0X_get_memory(). What do you 
think about these next patches?

Furthermore, returning PFlashCFI0X can be downcasted which seems safer than 
upcasting from DeviceState.

Best regards,
Bernhard

>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   hw/arm/gumstix.c     | 18 ++++++------------
>>   hw/arm/mainstone.c   | 13 +++++--------
>>   hw/arm/omap_sx1.c    | 22 ++++++++--------------
>>   hw/arm/versatilepb.c |  6 ++----
>>   hw/arm/z2.c          |  9 +++------
>>   hw/ppc/sam460ex.c    | 12 ++++--------
>>   6 files changed, 28 insertions(+), 52 deletions(-)