qemu-block
[Top][All Lists]
Advanced

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

Re: [PATCH v2 26/44] qom: Put name parameter before value / visitor para


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH v2 26/44] qom: Put name parameter before value / visitor parameter
Date: Fri, 3 Jul 2020 21:06:24 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0

03.07.2020 21:05, Vladimir Sementsov-Ogievskiy wrote:
02.07.2020 18:49, Markus Armbruster wrote:
The object_property_set_FOO() setters take property name and value in
an unusual order:

     void object_property_set_FOO(Object *obj, FOO_TYPE value,
                                  const char *name, Error **errp)

Having to pass value before name feels grating.  Swap them.

Same for object_property_set(), object_property_get(), and
object_property_parse().

Convert callers with this Coccinelle script:

     @@
     identifier fun = {object_property_get, object_property_parse, 
object_property_set_str, object_property_set_link, object_property_set_bool, 
object_property_set_int, object_property_set_uint, object_property_set, 
object_property_set_qobject};
     expression obj, v, name, errp;
     @@
     -    fun(obj, v, name, errp)
     +    fun(obj, name, v, errp)

I'd suggest

@@
identifier fun = {object_property_get, object_property_parse, 
object_property_set_str, object_property_set_link, object_property_set_bool, 
object_property_set_int, object_property_set_uint, object_property_set, 
object_property_set_qobject};
parameter obj, v, name, errp;
@@
-    fun(obj, v, name, errp)
+    fun(obj, name, v, errp)
      {...}


in addition, to not

do it by hand I mean



Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error
message "no position information".  Convert that one manually.

Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by
ARMSSE being used both as typedef and function-like macro there.
Convert manually.

Fails to convert hw/rx/rx-gdbsim.c, because Coccinelle gets confused
by RXCPU being used both as typedef and function-like macro there.
Convert manually.  The other files using RXCPU that way don't need
conversion.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>

The change should be safe, as compiler will complain if something is not 
updated correspondingly. The only exclusion are object_property_parse and 
object_property_set_str, where both key and value are strings. Check them 
presizely looking at

  vimdiff <(git grep object_property_parse HEAD^ | sed 's/HEAD\^://') <(git 
grep object_property_parse)

and

  vimdiff <(git grep -A 1 object_property_set_str HEAD^ | sed 's/HEAD\^://') 
<(git grep -A 1 object_property_set_str)

seems everything is updated.

Also, looked through manual changes for hw/arm/musicpal.c, hw/arm/armsse.c and 
hw/rx/rx-gdbsim.c. Seems correct..


---
  include/hw/audio/pcspk.h                 |   2 +-
  include/qom/object.h                     |  45 ++++-----
  include/qom/qom-qobject.h                |   7 +-
  backends/cryptodev.c                     |   2 +-

[..]

  static void property_release_tm(Object *obj, const char *name,
@@ -2384,10 +2375,8 @@ static void property_set_uint8_ptr(Object *obj, Visitor 
*v, const char *name,
  {
      uint8_t *field = opaque;
      uint8_t value;
-    Error *local_err = NULL;

This (and some more) chunks are obviously from some another patch..

-    if (!visit_type_uint8(v, name, &value, &local_err)) {
-        error_propagate(errp, local_err);
+    if (!visit_type_uint8(v, name, &value, errp)) {
          return;
      }


To find problems like this, I'm trying to rerun your cocci-script, but I don't 
know how exactly do you run it.

I've tried --use-gitgrep, but it doesn't find some files.

I've tried
git grep -l 
'object_property_get\|object_property_parse\|object_property_set_str\|object_property_set_link\|object_property_set_bool\|object_property_set_int\|object_property_set_uint\|object_property_set\|object_property_set_qobject'
 | xargs spatch script.cocci --macro-file scripts/cocci-macro-file.h --in-place 
--no-show-diff

Still, have not updated target/arm/monitor.c, strange..

Another fact, which makes it hard to check the patch is a lot of manual 
wrapping/indenting updates.. Hmm, I need some tool or script to make it simple 
to compare commits ignoring wrapping and indentation differences.



--
Best regards,
Vladimir



reply via email to

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