Max Filippov <[email protected]> writes: > On Mon, Feb 18, 2019 at 5:07 AM Markus Armbruster <[email protected]> wrote: >> >> pflash_cfi01_register() creates a TYPE_CFI_PFLASH01 device, sets >> properties, realizes, and wires up. >> >> We have three modified copies of it, because their users need to set >> additional properties, or have the wiring done differently. >> >> Factor out their common part into pflash_cfi01_create(). >> >> Signed-off-by: Markus Armbruster <[email protected]> >> --- >> hw/arm/vexpress.c | 22 +++++----------------- >> hw/arm/virt.c | 26 +++++++++----------------- >> hw/block/pflash_cfi01.c | 39 +++++++++++++++++++++++++++------------ >> hw/xtensa/xtfpga.c | 18 +++++++----------- > > I was told that it's better this way when I did that initially: > http://lists.gnu.org/archive/html/qemu-devel/2015-09/msg06927.html > > Has the idea of "better" changed since then? > I'm fine with the code either way, just curious.
I'm not sure about our collective idea of "better". I simply saw a helper function duplicated with minor variations because it fails to capture what's truly common, so I fixed that. If we think helper functions capturing device creation and property setting are bad, and open coding would be better. then we should get rid of pflash_cfi01_register() and pflash_cfi02_register() everywhere, not just avoid it in these three places. Cc'ing the usual QOM suspects for guidance.
