[PATCH 3/3] hw/i2c/pmbus_device: Fix modifying QOM class internals from instance

Philippe Mathieu-Daudé posted 3 patches 2 years, 8 months ago
Maintainers: Titus Rwantare <titusr@google.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>
[PATCH 3/3] hw/i2c/pmbus_device: Fix modifying QOM class internals from instance
Posted by Philippe Mathieu-Daudé 2 years, 8 months ago
QOM object instance should not modify its class state (because
all other objects instanciated from this class get affected).

Instead of modifying the PMBusDeviceClass 'device_num_pages' field
the first time a instance is initialized (in pmbus_pages_alloc),
introduce a new pmbus_pages_num() helper which returns the page
number from the class without modifying the class state.

The code logic become slighly simplified.

Inspired-by: Bernhard Beschow <shentey@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i2c/pmbus_device.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index 44fe4eddbb..8bc9d5108a 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -190,15 +190,18 @@ static void pmbus_quick_cmd(SMBusDevice *smd, uint8_t read)
     }
 }
 
+static uint8_t pmbus_pages_num(PMBusDevice *pmdev)
+{
+    const PMBusDeviceClass *k = PMBUS_DEVICE_GET_CLASS(pmdev);
+
+    /* some PMBus devices don't use the PAGE command, so they get 1 page */
+    return k->device_num_pages ? : 1;
+}
+
 static void pmbus_pages_alloc(PMBusDevice *pmdev)
 {
-    /* some PMBus devices don't use the PAGE command, so they get 1 page */
-    PMBusDeviceClass *k = PMBUS_DEVICE_GET_CLASS(pmdev);
-    if (k->device_num_pages == 0) {
-        k->device_num_pages = 1;
-    }
-    pmdev->num_pages = k->device_num_pages;
-    pmdev->pages = g_new0(PMBusPage, k->device_num_pages);
+    pmdev->num_pages = pmbus_pages_num(pmdev);
+    pmdev->pages = g_new0(PMBusPage, pmdev->num_pages);
 }
 
 void pmbus_check_limits(PMBusDevice *pmdev)
-- 
2.38.1


Re: [PATCH 3/3] hw/i2c/pmbus_device: Fix modifying QOM class internals from instance
Posted by Richard Henderson 2 years, 8 months ago
On 5/22/23 23:44, Philippe Mathieu-Daudé wrote:
> QOM object instance should not modify its class state (because
> all other objects instanciated from this class get affected).
> 
> Instead of modifying the PMBusDeviceClass 'device_num_pages' field
> the first time a instance is initialized (in pmbus_pages_alloc),
> introduce a new pmbus_pages_num() helper which returns the page
> number from the class without modifying the class state.
> 
> The code logic become slighly simplified.
> 
> Inspired-by: Bernhard Beschow<shentey@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/i2c/pmbus_device.c | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~