| Message ID | 20200203155412.7706-1-felipe@nutanix.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Improve default object property_add uint helpers | expand |
Oops, I completely forgot to add a "v5" on the subject line. (The changelog is there.) Let me know if I should resend. F. > On Feb 3, 2020, at 3:54 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 4 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 | 48 ++++++++-- > memory.c | 15 +-- > qom/object.c | 214 ++++++++++++++++++++++++++++++++++++++----- > target/arm/cpu.c | 22 +---- > target/i386/sev.c | 106 ++------------------- > ui/console.c | 4 +- > 14 files changed, 283 insertions(+), 319 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 > v4->v5: > - Rebase on latest master > - Available here: https://github.com/franciozzy/qemu/tree/autosetters
Hi On Mon, Feb 3, 2020 at 5:08 PM Felipe Franciosi <felipe@nutanix.com> wrote: > > Oops, I completely forgot to add a "v5" on the subject line. > > (The changelog is there.) > > Let me know if I should resend. > > F. > > > On Feb 3, 2020, at 3:54 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 4 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 | 48 ++++++++-- > > memory.c | 15 +-- > > qom/object.c | 214 ++++++++++++++++++++++++++++++++++++++----- > > target/arm/cpu.c | 22 +---- > > target/i386/sev.c | 106 ++------------------- > > ui/console.c | 4 +- > > 14 files changed, 283 insertions(+), 319 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 > > v4->v5: > > - Rebase on latest master > > - Available here: https://github.com/franciozzy/qemu/tree/autosetters Thanks for the rebase, it looks good overall, but: qom/object.c:2707:1: error: control reaches end of non-void function [-Werror=return-type]
> On Feb 3, 2020, at 4:10 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > > Hi > > On Mon, Feb 3, 2020 at 5:08 PM Felipe Franciosi <felipe@nutanix.com> wrote: >> >> Oops, I completely forgot to add a "v5" on the subject line. >> >> (The changelog is there.) >> >> Let me know if I should resend. >> >> F. >> >>> On Feb 3, 2020, at 3:54 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 4 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 | 48 ++++++++-- >>> memory.c | 15 +-- >>> qom/object.c | 214 ++++++++++++++++++++++++++++++++++++++----- >>> target/arm/cpu.c | 22 +---- >>> target/i386/sev.c | 106 ++------------------- >>> ui/console.c | 4 +- >>> 14 files changed, 283 insertions(+), 319 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 >>> v4->v5: >>> - Rebase on latest master >>> - Available here: https://github.com/franciozzy/qemu/tree/autosetters > > Thanks for the rebase, it looks good overall, but: > > qom/object.c:2707:1: error: control reaches end of non-void function > [-Werror=return-type] That was an oversight. :) Let me fix it and send a corrected (v6) subject line. Felipe > > > -- > Marc-André Lureau