Re: [PATCH 3/4] crypto: use QOM macros for declaration/definition of secret types

[Eduardo Habkost <ehabkost@redhat.com>]

On Fri, Aug 07, 2020 at 12:11:48PM +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 06, 2020 at 02:01:54PM -0400, Eduardo Habkost wrote:
> > On Fri, Jul 24, 2020 at 10:12:45AM +0100, Daniel P. Berrangé wrote:
> > > On Thu, Jul 23, 2020 at 02:50:06PM -0400, Eduardo Habkost wrote:
> > > > On Thu, Jul 23, 2020 at 07:14:09PM +0100, Daniel P. Berrangé wrote:
> > > > > This introduces the use of the OBJECT_DEFINE and OBJECT_DECLARE macro
> > > > > families in the secret types, in order to eliminate boilerplate code.
> > > > > 
> > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > > > ---
> > > > >  crypto/secret.c                 | 24 ++++--------------------
> > > > >  crypto/secret_common.c          | 32 +++++++++-----------------------
> > > > >  crypto/secret_keyring.c         | 28 +++++++++-------------------
> > > > >  include/crypto/secret.h         | 11 ++---------
> > > > >  include/crypto/secret_common.h  | 13 ++-----------
> > > > >  include/crypto/secret_keyring.h | 18 ++----------------
> > > > >  6 files changed, 28 insertions(+), 98 deletions(-)
> > > > > 
> > > > 
> > > > Beautiful.
> > > > 
> > > > I wonder how hard it would be to automate this.  I'm assuming
> > > > Coccinelle won't be able to deal with the macro definitions, but
> > > > a handwritten conversion script would be really useful for
> > > > dealing with our 1226 static TypeInfo structs.
> > > 
> > > Probably possible to do a reasonably good job with a perl script or
> > > similar. The code patterns to be replaced are reasonably easy to
> > > identify with a few regexes.
> > 
> > I've attempted to parse all the TypeInfo structs in the tree.
> > The data I've extracted is available at:
> > https://gist.github.com/ehabkost/7a398640492f369685c789ffed0f67aa
> > 
> > It turns out 230 of our 1259 TypeInfo variables don't have
> > instance_size set and don't have their own struct type defined.
> > 
> > We could:
> > * Make that a supported use case, and add helper macros that don't
> >   require MyDevice to be defined;
> > * Make that not supported, and convert those 230 types automatically; or
> > * Make that not supported, and convert those 230 types manually.
> 
> When we force an instance struct, we also force definition of an
> instance init and finalize function.
> 
> 230 types is probably enough to justify a further macro that allows
> the instance struct, init & finalize funtions to be omitted.

Status update: the TypeInfo parser evolved to become a converter
able to replace the type checking macros (automatic conversion of
TypeInfo declarations will be done soon).

https://github.com/ehabkost/qemu-hacks/commits/work/qom-macros-autoconvert

Additional obstacles we'll need to address:

- Sometimes the struct typedefs are in a completely different
  file from the type checking macros.  I've worked around this
  problem by introducing macros that will only add the type
  casting functions, but no typedefs.
- There's some usage of const object pointers in the code,
  which breaks the new type cast functions:
  https://travis-ci.org/github/ehabkost/qemu-hacks/jobs/716033062#L1417

We can probably use _Generic to make the type cast functions
const-safe, but I'm sure this will break existing code that
expects the type casts to always return non-const pointers.

-- 
Eduardo