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

Felipe Franciosi posted 4 patches 4 years, 4 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
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(-)
[PATCH v4 0/4] Improve default object property_add uint helpers
Posted by Felipe Franciosi 4 years, 4 months ago
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(-)

-- 
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
Re: [PATCH v4 0/4] Improve default object property_add uint helpers
Posted by Marc-André Lureau 4 years, 4 months ago
Hi

On Thu, Dec 19, 2019 at 10:02 PM Felipe Franciosi <felipe@nutanix.com> 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 <marcandre.lureau@redhat.com>

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

Re: [PATCH v4 0/4] Improve default object property_add uint helpers
Posted by Felipe Franciosi 4 years, 4 months ago

> On Dec 20, 2019, at 3:15 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> 
> Hi
> 
> On Thu, Dec 19, 2019 at 10:02 PM Felipe Franciosi <felipe@nutanix.com> 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 <marcandre.lureau@redhat.com>

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

Re: [PATCH v4 0/4] Improve default object property_add uint helpers
Posted by Felipe Franciosi 4 years, 2 months ago
Hi Marc-Andre and Paolo,

> On Dec 20, 2019, at 3:15 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> 
> Hi
> 
> On Thu, Dec 19, 2019 at 10:02 PM Felipe Franciosi <felipe@nutanix.com> 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 <marcandre.lureau@redhat.com>
> 
> Paolo, is it going through your queue?

I didn't see any response after this. Did the series get lost?

Cheers,
Felipe

> 
>> 
>> --
>> 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

Re: [PATCH v4 0/4] Improve default object property_add uint helpers
Posted by Marc-André Lureau 4 years, 2 months ago
Hi Felipe,

On Fri, Jan 24, 2020 at 11:49 AM Felipe Franciosi <felipe@nutanix.com> wrote:
>
> Hi Marc-Andre and Paolo,
>
> > On Dec 20, 2019, at 3:15 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> >
> > Hi
> >
> > On Thu, Dec 19, 2019 at 10:02 PM Felipe Franciosi <felipe@nutanix.com> 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 <marcandre.lureau@redhat.com>
> >
> > Paolo, is it going through your queue?
>
> I didn't see any response after this. Did the series get lost?

Can you send a rebased version?

thanks



-- 
Marc-André Lureau

Re: [PATCH v4 0/4] Improve default object property_add uint helpers
Posted by Felipe Franciosi 4 years, 2 months ago
Heya,

> On Jan 28, 2020, at 3:54 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> 
> Hi Felipe,
> 
> On Fri, Jan 24, 2020 at 11:49 AM Felipe Franciosi <felipe@nutanix.com> wrote:
>> 
>> Hi Marc-Andre and Paolo,
>> 
>>> On Dec 20, 2019, at 3:15 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
>>> 
>>> Hi
>>> 
>>> On Thu, Dec 19, 2019 at 10:02 PM Felipe Franciosi <felipe@nutanix.com> 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 <marcandre.lureau@redhat.com>
>>> 
>>> Paolo, is it going through your queue?
>> 
>> I didn't see any response after this. Did the series get lost?
> 
> Can you send a rebased version?

Sorry for the delay as I was on travels. Done.

F.

> 
> thanks
> 
> 
> 
> -- 
> Marc-André Lureau

Re: [PATCH v4 0/4] Improve default object property_add uint helpers
Posted by Alexey Kardashevskiy 4 years, 4 months ago

On 20/12/2019 05:02, Felipe Franciosi 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.

A weird thing happens - when I apply patches from my mailer (thunderbird
-> open the source -> cut-n-paste to "git am") - they fail to apply. And
the mails themselves look suspicious - too many "MS-Exchange" and
"X-Proofpoint" :)

A bundle from
https://patchwork.ozlabs.org/project/qemu-devel/list/?series=149673
applies fine though.


Anyway, this works on powerpc. Thanks,



> 
> 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(-)
> 

-- 
Alexey

Re: [PATCH v4 0/4] Improve default object property_add uint helpers
Posted by Felipe Franciosi 4 years, 4 months ago
Hi,

> On Dec 19, 2019, at 11:56 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
> 
> 
> On 20/12/2019 05:02, Felipe Franciosi 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.
> 
> A weird thing happens - when I apply patches from my mailer (thunderbird
> -> open the source -> cut-n-paste to "git am") - they fail to apply. And
> the mails themselves look suspicious - too many "MS-Exchange" and
> "X-Proofpoint" :)

I apologise for that... as you can see from below, our company's
"anti-spam" / "anti-virus" mail servers tend to mangle incoming
e-mails in ways that make it challenging to work with MLs:

> 
> A bundle from
> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_qemu-2Ddevel_list_-3Fseries-3D149673&d=DwICaQ&c=s883GpUCOChKOHiocYtGcg&r=CCrJKVC5zGot8PrnI-iYe00MdX4pgdQfMRigp14Ptmk&m=TxZxCvPIyfiAMkXeqOwUO_oYhAWFHAG66jA3SeEldzU&s=vMLzvHMGCJ9VeW3wSKvJVPrthKmVoud-ACf5eh6w2Rg&e= 
> applies fine though.

Now I know why both you and Marc-Andre had problems applying my
patches: apparently our servers also mangle with outgoing e-mails too.

I heard that pulling the patches from mbox work, but I'll make sure to
post patches on Github in the future to make things easier for others.
I've already asked them to look into this and whitelist e-mails on
various criteria.

Thanks for your help in making sure these patches work for you despite
the extra hurdles!

F.

> 
> 
> Anyway, this works on powerpc. Thanks,
> 
> 
> 
>> 
>> 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(-)
>> 
> 
> -- 
> Alexey


Re: [PATCH v4 0/4] Improve default object property_add uint helpers
Posted by Stefan Hajnoczi 4 years, 4 months ago
On Thu, Dec 19, 2019 at 06:02:28PM +0000, Felipe Franciosi 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(-)
> 
> -- 
> 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

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>