Re: [RFC PATCH 2/3] hw/i2c/smbus_eeprom: Add description based on child name

[Eduardo Habkost]

On Fri, Jun 26, 2020 at 04:26:33PM +0200, Philippe Mathieu-Daudé wrote:
> On 6/26/20 1:00 PM, BALATON Zoltan wrote:
> > On Fri, 26 Jun 2020, Philippe Mathieu-Daudé wrote:
> >> Suggested-by: Markus Armbruster <armbru@redhat.com>
> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >> ---
> >> hw/i2c/smbus_eeprom.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
> >> index 879fd7c416..22ba7b20d4 100644
> >> --- a/hw/i2c/smbus_eeprom.c
> >> +++ b/hw/i2c/smbus_eeprom.c
> >> @@ -47,6 +47,7 @@ typedef struct SMBusEEPROMDevice {
> >> Â Â Â  uint8_t *init_data;
> >> Â Â Â  uint8_t offset;
> >> Â Â Â  bool accessed;
> >> +Â Â Â  char *description;
> >> } SMBusEEPROMDevice;
> >>
> >> static uint8_t eeprom_receive_byte(SMBusDevice *dev)
> >> @@ -134,7 +135,9 @@ static void smbus_eeprom_realize(DeviceState *dev,
> >> Error **errp)
> >> Â Â Â  smbus_eeprom_reset(dev);
> >> Â Â Â  if (eeprom->init_data == NULL) {
> >> Â Â Â Â Â Â Â  error_setg(errp, "init_data cannot be NULL");
> >> +Â Â Â Â Â Â Â  return;
> >> Â Â Â  }
> >> +Â Â Â  eeprom->description =
> >> object_get_canonical_path_component(OBJECT(dev));
> >> }
> >>
> >> static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data)
> > 
> > What is this for? Shouldn't this description field be in some parent
> > object and whatever wants to print it could use the
> > object_get_canonical_path_component() as default value if it's not set
> > instead of adding more boiler plate to every object?
> 
> You are right, if we want to use this field generically, it should be
> a static Object field. I'll defer that question to Eduardo/Markus.

I don't understand what's the question here.  What would be the
purpose of a static Object field, and why it would be better than
simply calling object_get_canonical_path_component() when you
need it?

-- 
Eduardo