Kevin Wolf <kw...@redhat.com> writes:
> Am 24.04.2014 um 11:15 hat Markus Armbruster geschrieben:
>> error_is_set(&var) is the same as var != NULL, but it takes
>> whole-program analysis to figure that out. Unnecessarily hard for
>> optimizers, static checkers, and human readers. Commit 84d18f0 dumbed
>> it down to obvious, but a few more have crept in since, and
>> documentation was overlooked. Dumb these down, too.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> block/mirror.c | 2 +-
>> block/nfs.c | 2 +-
>> block/quorum.c | 4 ++--
>> docs/writing-qmp-commands.txt | 6 +++---
>> tests/test-qmp-input-strict.c | 8 ++++----
>> 5 files changed, 11 insertions(+), 11 deletions(-)
>
>> diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
>> index 8349dec..3930a9b 100644
>> --- a/docs/writing-qmp-commands.txt
>> +++ b/docs/writing-qmp-commands.txt
>> @@ -311,7 +311,7 @@ void hmp_hello_world(Monitor *mon, const QDict *qdict)
>> Error *errp = NULL;
>>
>> qmp_hello_world(!!message, message, &errp);
>> - if (error_is_set(&errp)) {
>> + if (errp) {
>> monitor_printf(mon, "%s\n", error_get_pretty(errp));
>> error_free(errp);
>> return;
>> @@ -483,7 +483,7 @@ void hmp_info_alarm_clock(Monitor *mon)
>> Error *errp = NULL;
>>
>> clock = qmp_query_alarm_clock(&errp);
>> - if (error_is_set(&errp)) {
>> + if (errp) {
>> monitor_printf(mon, "Could not query alarm clock information\n");
>> error_free(errp);
>> return;
>> @@ -634,7 +634,7 @@ void hmp_info_alarm_methods(Monitor *mon)
>> Error *errp = NULL;
>>
>> method_list = qmp_query_alarm_methods(&errp);
>> - if (error_is_set(&errp)) {
>> + if (errp) {
>> monitor_printf(mon, "Could not query alarm methods\n");
>> error_free(errp);
>> return;
>> diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
>> index 38b5e95..f03353b 100644
>> --- a/tests/test-qmp-input-strict.c
>> +++ b/tests/test-qmp-input-strict.c
>> @@ -153,7 +153,7 @@ static void
>> test_validate_union_flat(TestInputVisitorData *data,
>> /* TODO when generator bug is fixed, add 'integer': 41 */
>>
>> visit_type_UserDefFlatUnion(v, &tmp, NULL, &errp);
>> - g_assert(!error_is_set(&errp));
>> + g_assert(!errp);
>> qapi_free_UserDefFlatUnion(tmp);
>> }
>>
>> @@ -167,7 +167,7 @@ static void
>> test_validate_union_anon(TestInputVisitorData *data,
>> v = validate_test_init(data, "42");
>>
>> visit_type_UserDefAnonUnion(v, &tmp, NULL, &errp);
>> - g_assert(!error_is_set(&errp));
>> + g_assert(!errp);
>> qapi_free_UserDefAnonUnion(tmp);
>> }
>>
>> @@ -240,7 +240,7 @@ static void
>> test_validate_fail_union_flat(TestInputVisitorData *data,
>> v = validate_test_init(data, "{ 'string': 'c', 'integer': 41,
>> 'boolean': true }");
>>
>> visit_type_UserDefFlatUnion(v, &tmp, NULL, &errp);
>> - g_assert(error_is_set(&errp));
>> + g_assert(errp);
>> qapi_free_UserDefFlatUnion(tmp);
>> }
>>
>> @@ -254,7 +254,7 @@ static void
>> test_validate_fail_union_anon(TestInputVisitorData *data,
>> v = validate_test_init(data, "3.14");
>>
>> visit_type_UserDefAnonUnion(v, &tmp, NULL, &errp);
>> - g_assert(error_is_set(&errp));
>> + g_assert(errp);
>> qapi_free_UserDefAnonUnion(tmp);
>> }
>
> Not directly related to your change, but in the documentation and test
> s/errp/local_err/ would help the consistency. When I read 'errp' my
> first association is an Error** argument that may be NULL.
My error_is_set development branch contains tree-wide cleanup to use
errp only for Error **, and err/local_err only for Error *. I split it
up some (see 2/5) to avoid the usual fate of my tree-wide cleanups :)
