qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] Object cast macro change-pattern automation.


From: Andreas Färber
Subject: Re: [Qemu-devel] Object cast macro change-pattern automation.
Date: Fri, 21 Jun 2013 11:33:08 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6

Hi Peter,

Am 21.06.2013 06:21, schrieb Peter Crosthwaite:
> I thought Id share with you a little script I made (not very polished)
> that I used to help with some of my patches creating the QOM cast
> macros (mainly the PCI ones). May be useful in speeding up the
> QOMification effort. Andreas, im guessing you may have something
> similar going if your able to comment?

In fact my conversion patches both for QOM and for CPUState were all
hand-tailored, checking with git-grep for missed occurrences. My focus
was on the cast macros though, with TYPE_* being useful since reused
inside the cast macro. Sometimes I use gEdit's Search-and-replace dialog
or similar tools that let me visually verify before changing.

Were your PCI conversions already scripted? In that case we should take
a closer look for false positives. CC'ing mst.

> I know Hu mentioned he wanted
> to work on QOMification of sysbus - which is a big job so stuff like
> this may make life easier.

That's great to hear. I'm wondering how to best go about that - would
there be interest in reviving my qom-next tree for staging?

Problem I see is that these series are equally touching on maintained
and not actively maintained devices and that on the one hand some
maintainers are ignorant of these QOM concepts and can't really review
(but should still help test for regressions) whereas on the other hand
other contributed functional patches may conflict with the refactorings.

Also some coordination to avoid duplicate conversions might make sense.

> example usage:
> 
> $ source ./object_macro_maker hw/timer/xilinx_timer.c XILINX_TIMER
> 
> 1st arg is target file, 2 arg is the name of the type, I.e. FOO in TYPE_FOO
> 
> It will automatically find replace usages of the string literal type
> inplace and give you a fragment to copy-paste into the source defining
> the type string and object cast macro.
> 
> It has the limitation that it only works with files that define a
> single QOM type. I didnt bother trying to generalise as such files are
> the exception and not the rule.

Depending on the specific case I believe it may make sense to extract
devices from such a large file, especially now with the restructured hw/
directory. ADB is one such case where I started looking into it; other
examples would be SBus, MusicPal and e500.

> 
> Example output below:
> 
> diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
> index 0c39cff..ae09170 100644
> --- a/hw/timer/xilinx_timer.c
> +++ b/hw/timer/xilinx_timer.c
> @@ -218,7 +218,7 @@ static int xilinx_timer_init(SysBusDevice *dev)
>          ptimer_set_freq(xt->ptimer, t->freq_hz);
>      }
> 
> -    memory_region_init_io(&t->mmio, &timer_ops, t, "xlnx.xps-timer",
> +    memory_region_init_io(&t->mmio, &timer_ops, t, TYPE_XILINX_TIMER,
>                            R_MAX * 4 * num_timers(t));
>      sysbus_init_mmio(dev, &t->mmio);
>      return 0;
> @@ -241,7 +241,7 @@ static void xilinx_timer_class_init(ObjectClass
> *klass, void *data)
>  }
> 
>  static const TypeInfo xilinx_timer_info = {
> -    .name          = "xlnx.xps-timer",
> +    .name          = TYPE_XILINX_TIMER,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(struct timerblock),
>      .class_init    = xilinx_timer_class_init,
> State Struct is struct timerblock
> ------------------ cut here ------------------------
> #define TYPE_XILINX_TIMER "xlnx.xps-timer"
> 
> #define XILINX_TIMER(obj) \
>     OBJECT_CHECK(struct timerblock, (obj), TYPE_XILINX_TIMER)
> -----------------------------------------------------------------

That's functionally correct, but when occurrences were low I've tried to
fix CamelCase and lack of typedef as well. Could be done in a separate
patch, with a different script of course.

Cheers,
Andreas

> 
> 
> And the script itself:
> 
> #!/bin/bash
> 
> sed -n '/^static const TypeInfo.*$/,/^};.*$/p' $1 | \
>                         grep "\(\.instance_size\|\.name\)"\
>                         > typeinfo.tmp
> cat typeinfo.tmp
> STRING=$(grep -o "\".*\"" typeinfo.tmp | sed 's/\"//g')
> 
> echo "String is ${STRING}"
> sed "s/\"${STRING}\"/TYPE_${2}/g" -i ${1}
> git diff ${1} | cat
> 
> STATE_STRUCT=$(grep -o "(.*)" typeinfo.tmp | sed "s/(//" | sed "s/)//")
> echo "State Struct is ${STATE_STRUCT}"
> echo "------------------ cut here ------------------------"
> 
> echo "#define TYPE_${2} \"${STRING}\""
> echo ""
> echo "#define ${2}(obj) \\"
> echo "    OBJECT_CHECK(${STATE_STRUCT}, (obj), TYPE_${2})"
> 
> 
> Regards,
> Peter
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



reply via email to

[Prev in Thread] Current Thread [Next in Thread]