Make qbus_list_bus and qbus_list_dev hint append helpers well formed:
switch errp paramter to Error *const * type, as it has uncommon
behavior: not change the pointer to return error, but operate on
already existent error object.
Rename functions to be error_append_*_hint.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qdev-monitor.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index 29ed73e56a..3465a1e2d0 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -328,7 +328,8 @@ static Object *qdev_get_peripheral_anon(void)
     return dev;
 }
 
-static void qbus_list_bus(DeviceState *dev, Error **errp)
+static void qbus_error_append_bus_list_hint(DeviceState *dev,
+                                            Error *const *errp)
 {
     BusState *child;
     const char *sep = " ";
@@ -342,7 +343,8 @@ static void qbus_list_bus(DeviceState *dev, Error **errp)
     error_append_hint(errp, "\n");
 }
 
-static void qbus_list_dev(BusState *bus, Error **errp)
+static void qbus_error_append_dev_list_hint(BusState *bus,
+                                            Error *const *errp)
 {
     BusChild *kid;
     const char *sep = " ";
@@ -500,7 +502,7 @@ static BusState *qbus_find(const char *path, Error **errp)
         if (!dev) {
             error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
                       "Device '%s' not found", elem);
-            qbus_list_dev(bus, errp);
+            qbus_error_append_dev_list_hint(bus, errp);
             return NULL;
         }
 
@@ -518,7 +520,7 @@ static BusState *qbus_find(const char *path, Error **errp)
             if (dev->num_child_bus) {
                 error_setg(errp, "Device '%s' has multiple child buses",
                            elem);
-                qbus_list_bus(dev, errp);
+                qbus_error_append_bus_list_hint(dev, errp);
             } else {
                 error_setg(errp, "Device '%s' has no child bus", elem);
             }
@@ -534,7 +536,7 @@ static BusState *qbus_find(const char *path, Error **errp)
         bus = qbus_find_bus(dev, elem);
         if (!bus) {
             error_setg(errp, "Bus '%s' not found", elem);
-            qbus_list_bus(dev, errp);
+            qbus_error_append_bus_list_hint(dev, errp);
             return NULL;
         }
     }
-- 
2.21.0

On 12/5/19 11:46 AM, Vladimir Sementsov-Ogievskiy wrote:
> Make qbus_list_bus and qbus_list_dev hint append helpers well formed:
> switch errp paramter to Error *const * type, as it has uncommon
> behavior: not change the pointer to return error, but operate on
> already existent error object.
> Rename functions to be error_append_*_hint.

qbus_error_append_*_hint

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   qdev-monitor.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 

> -static void qbus_list_dev(BusState *bus, Error **errp)
> +static void qbus_error_append_dev_list_hint(BusState *bus,
> +                                            Error *const *errp)

Or, since you copy-and-paste this commit message in v9, you could use 
*_error_append_*_hint between the commits for less one-off fixing per 
message.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On 12/5/19 12:17 PM, Eric Blake wrote:
> On 12/5/19 11:46 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Make qbus_list_bus and qbus_list_dev hint append helpers well formed:
>> switch errp paramter to Error *const * type, as it has uncommon
>> behavior: not change the pointer to return error, but operate on
>> already existent error object.
>> Rename functions to be error_append_*_hint.
> 
> qbus_error_append_*_hint
> 
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   qdev-monitor.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
> 
>> -static void qbus_list_dev(BusState *bus, Error **errp)
>> +static void qbus_error_append_dev_list_hint(BusState *bus,
>> +                                            Error *const *errp)
> 
> Or, since you copy-and-paste this commit message in v9, you could use 

Sorry, I meant it was copied in v8 7/21.

> *_error_append_*_hint between the commits for less one-off fixing per 
> message.
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On 12/5/19 6:46 PM, Vladimir Sementsov-Ogievskiy wrote:
> Make qbus_list_bus and qbus_list_dev hint append helpers well formed:
> switch errp paramter to Error *const * type, as it has uncommon

"parameter"

> behavior: not change the pointer to return error, but operate on
> already existent error object.
> Rename functions to be error_append_*_hint.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> Make qbus_list_bus and qbus_list_dev hint append helpers well formed:
> switch errp paramter to Error *const * type, as it has uncommon
> behavior: not change the pointer to return error, but operate on
> already existent error object.
> Rename functions to be error_append_*_hint.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

"well form error hint helpers" tells me nothing.  PATCH 03 does the same
(less function renames), and is titled "make Error **errp const where it
is appropriate".  I like that better.  No big deal, and certainly not
worth a respin by itself.  I'm happy to touch up commit messages in my
tree.

Markus Armbruster <armbru@redhat.com> writes:

> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>
>> Make qbus_list_bus and qbus_list_dev hint append helpers well formed:
>> switch errp paramter to Error *const * type, as it has uncommon
>> behavior: not change the pointer to return error, but operate on
>> already existent error object.
>> Rename functions to be error_append_*_hint.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> "well form error hint helpers" tells me nothing.  PATCH 03 does the same
> (less function renames), and is titled "make Error **errp const where it
> is appropriate".  I like that better.  No big deal, and certainly not
> worth a respin by itself.  I'm happy to touch up commit messages in my
> tree.

Let's adapt PATCH 03's commit message, like this:

  qdev-monitor: make Error **errp const where it is appropriate

  Mostly, Error ** is for returning error from the function, so the
  callee sets it. However qbus_list_bus and qbus_list_dev get already
  filled errp parameter. They don't change the pointer itself, only
  change the internal state of referenced Error object. So we can make
  it Error *const * errp, to stress the behavior. It will also help
  coccinelle script (in future) to distinguish such cases from common
  errp usage.

  While there, rename the functions to
  qbus_error_append_bus_list_hint(), qbus_error_append_dev_list_hint().

With something like that:
Reviewed-by: Markus Armbruster <armbru@redhat.com>