On Fri, Nov 22, 2019 at 3:07 PM Philippe Mathieu-Daudé
<phi...@redhat.com> wrote:
>
> On 11/20/19 4:24 PM, Marc-André Lureau wrote:
> > Instead, set the initial data field directly.
> >
> > (the initial data is an array of 256 bytes. As I don't know if it may
> > change over time, I keep the pointer to original buffer as is, but it
> > might be worth to consider to copy it instead)
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> > ---
> > hw/i2c/smbus_eeprom.c | 17 ++++++++---------
> > 1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
> > index 54c86a0112..533c728b3b 100644
> > --- a/hw/i2c/smbus_eeprom.c
> > +++ b/hw/i2c/smbus_eeprom.c
> > @@ -44,7 +44,7 @@
> > typedef struct SMBusEEPROMDevice {
> > SMBusDevice smbusdev;
> > uint8_t data[SMBUS_EEPROM_SIZE];
> > - void *init_data;
> > + uint8_t *init_data;
> > uint8_t offset;
> > bool accessed;
> > } SMBusEEPROMDevice;
> > @@ -129,14 +129,14 @@ static void smbus_eeprom_reset(DeviceState *dev)
> >
> > static void smbus_eeprom_realize(DeviceState *dev, Error **errp)
> > {
> > + SMBusEEPROMDevice *eeprom = SMBUS_EEPROM(dev);
> > +
> > smbus_eeprom_reset(dev);
> > + if (eeprom->init_data == NULL) {
> > + error_setg(errp, "init_data cannot be NULL");
> > + }
> > }
> >
> > -static Property smbus_eeprom_properties[] = {
> > - DEFINE_PROP_PTR("data", SMBusEEPROMDevice, init_data),
> > - DEFINE_PROP_END_OF_LIST(),
> > -};
> > -
> > static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data)
> > {
> > DeviceClass *dc = DEVICE_CLASS(klass);
> > @@ -146,9 +146,8 @@ static void smbus_eeprom_class_initfn(ObjectClass
> > *klass, void *data)
> > dc->reset = smbus_eeprom_reset;
> > sc->receive_byte = eeprom_receive_byte;
> > sc->write_data = eeprom_write_data;
> > - dc->props = smbus_eeprom_properties;
> > dc->vmsd = &vmstate_smbus_eeprom;
> > - /* Reason: pointer property "data" */
> > + /* Reason: init_data */
> > dc->user_creatable = false;
> > }
> >
> > @@ -172,7 +171,7 @@ void smbus_eeprom_init_one(I2CBus *smbus, uint8_t
> > address, uint8_t *eeprom_buf)
> >
> > dev = qdev_create((BusState *) smbus, TYPE_SMBUS_EEPROM);
> > qdev_prop_set_uint8(dev, "address", address);
> > - qdev_prop_set_ptr(dev, "data", eeprom_buf);
> > + SMBUS_EEPROM(dev)->init_data = eeprom_buf;
>
> Isn't this a QOM anti-pattern?
Sort of, but using prop_ptr in the first place is an anti-pattern. So
I am not improving the situation much here, but just getting rid of
prop_ptr usage.
Using an array of bytes instead could be done in a follow-up patch. I
can leave a fixme if that helps.
>
> > qdev_init_nofail(dev);
> > }
> >
> >
>
