Re: [PATCH v4 0/4] Improve default object property_add uint helpers

[Felipe Franciosi]

> On Dec 20, 2019, at 3:15 PM, Marc-André Lureau <address@hidden> wrote:
> 
> Hi
> 
> On Thu, Dec 19, 2019 at 10:02 PM Felipe Franciosi <address@hidden> wrote:
>> 
>> This improves the family of object_property_add_uintXX_ptr helpers by 
>> enabling
>> a default getter/setter only when desired. To prevent an API behavioural 
>> change
>> (from clients that already used these helpers and did not want a setter), we
>> add a OBJ_PROP_FLAG_READ flag that allow clients to only have a getter. 
>> Patch 1
>> enhances the API and modify current users.
>> 
>> While modifying the clients of the API, a couple of improvement opportunities
>> were observed in ich9. These were added in separate patches (2 and 3).
>> 
>> Patch 3 cleans up a lot of existing code by moving various objects to the
>> enhanced API. Previously, those objects had their own getters/setters that 
>> only
>> updated the values without further checks. Some of them actually lacked a 
>> check
>> for setting overflows, which could have resulted in undesired values being 
>> set.
>> The new default setters include a check for that, not updating the values in
>> case of errors (and propagating them). If they did not provide an error
>> pointer, then that behaviour was maintained.
>> 
>> Felipe Franciosi (4):
>>  qom/object: enable setter for uint types
>>  ich9: fix getter type for sci_int property
>>  ich9: Simplify ich9_lpc_initfn
>>  qom/object: Use common get/set uint helpers
>> 
>> hw/acpi/ich9.c       |  99 ++------------------
>> hw/acpi/pcihp.c      |   7 +-
>> hw/acpi/piix4.c      |  12 +--
>> hw/isa/lpc_ich9.c    |  27 ++----
>> hw/misc/edu.c        |  13 +--
>> hw/pci-host/q35.c    |  14 +--
>> hw/ppc/spapr.c       |  18 +---
>> hw/ppc/spapr_drc.c   |   3 +-
>> include/qom/object.h |  44 +++++++--
>> memory.c             |  15 +--
>> qom/object.c         | 216 ++++++++++++++++++++++++++++++++++++++-----
>> target/arm/cpu.c     |  22 +----
>> target/i386/sev.c    | 106 ++-------------------
>> ui/console.c         |   4 +-
>> 14 files changed, 282 insertions(+), 318 deletions(-)
> 
> It conflicts with some recent changes, so you'll need to send a new
> version, but that one looks good to me:
> Reviewed-by: Marc-André Lureau <address@hidden>

Ah, no worries.

I did do a rebase on top of master before sending out v4, though.
Which conflict did you see? I can rebase later or maybe whomever is
pulling can fix the merge if it's straightforward?

F.


> 
> Paolo, is it going through your queue?
> 
>> 
>> --
>> 2.20.1
>> 
>> Changelog:
>> v1->v2:
>> - Update sci_int directly instead of using stack variable
>> - Defining an enhanced ObjectPropertyFlags instead of just 'readonly'
>> - Erroring out directly (instead of using gotos) on default setters
>> - Retaining lack of errp passing when it wasn't there
>> v2->v3:
>> - Rename flags _RD to _READ and _WR to _WRITE
>> - Add a convenience _READWRITE flag
>> - Drop the usage of UL in the bit flag definitions
>> v3->v4:
>> - Drop changes to hw/vfio/pci-quirks.c
> 
> 
> 
> -- 
> Marc-André Lureau