Am 17. Oktober 2022 20:57:06 UTC schrieb "Philippe Mathieu-Daudé" <phi...@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 <shen...@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(-)
