[PATCH 01/46] error: Improve examples in error.h's big comment

Markus Armbruster posted 46 patches 5 years, 5 months ago
There is a newer version of this series
[PATCH 01/46] error: Improve examples in error.h's big comment
Posted by Markus Armbruster 5 years, 5 months ago
Show errp instead of &err where &err is actually unusual.  Add a
missing declaration.  Add a second error pileup example.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/error.h | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index ad5b6e896d..1a5ea25e12 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -16,15 +16,15 @@
  * Error reporting system loosely patterned after Glib's GError.
  *
  * Create an error:
- *     error_setg(&err, "situation normal, all fouled up");
+ *     error_setg(errp, "situation normal, all fouled up");
  *
  * Create an error and add additional explanation:
- *     error_setg(&err, "invalid quark");
- *     error_append_hint(&err, "Valid quarks are up, down, strange, "
+ *     error_setg(errp, "invalid quark");
+ *     error_append_hint(errp, "Valid quarks are up, down, strange, "
  *                       "charm, top, bottom.\n");
  *
  * Do *not* contract this to
- *     error_setg(&err, "invalid quark\n"
+ *     error_setg(errp, "invalid quark\n" // WRONG!
  *                "Valid quarks are up, down, strange, charm, top, bottom.");
  *
  * Report an error to the current monitor if we have one, else stderr:
@@ -108,12 +108,23 @@
  *     }
  *
  * Do *not* "optimize" this to
+ *     Error *err = NULL;
  *     foo(arg, &err);
  *     bar(arg, &err); // WRONG!
  *     if (err) {
  *         handle the error...
  *     }
  * because this may pass a non-null err to bar().
+ *
+ * Likewise, do *not*
+ *     Error *err = NULL;
+ *     if (cond1) {
+ *         error_setg(err, ...);
+ *     }
+ *     if (cond2) {
+ *         error_setg(err, ...); // WRONG!
+ *     }
+ * because this may pass a non-null err to error_setg().
  */
 
 #ifndef ERROR_H
-- 
2.26.2


Re: [PATCH 01/46] error: Improve examples in error.h's big comment
Posted by Greg Kurz 5 years, 5 months ago
On Wed, 24 Jun 2020 18:42:59 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Show errp instead of &err where &err is actually unusual.  Add a
> missing declaration.  Add a second error pileup example.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/error.h | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index ad5b6e896d..1a5ea25e12 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -16,15 +16,15 @@
>   * Error reporting system loosely patterned after Glib's GError.
>   *
>   * Create an error:
> - *     error_setg(&err, "situation normal, all fouled up");
> + *     error_setg(errp, "situation normal, all fouled up");
>   *
>   * Create an error and add additional explanation:
> - *     error_setg(&err, "invalid quark");
> - *     error_append_hint(&err, "Valid quarks are up, down, strange, "
> + *     error_setg(errp, "invalid quark");
> + *     error_append_hint(errp, "Valid quarks are up, down, strange, "
>   *                       "charm, top, bottom.\n");
>   *
>   * Do *not* contract this to
> - *     error_setg(&err, "invalid quark\n"
> + *     error_setg(errp, "invalid quark\n" // WRONG!
>   *                "Valid quarks are up, down, strange, charm, top, bottom.");
>   *
>   * Report an error to the current monitor if we have one, else stderr:
> @@ -108,12 +108,23 @@
>   *     }
>   *
>   * Do *not* "optimize" this to
> + *     Error *err = NULL;
>   *     foo(arg, &err);
>   *     bar(arg, &err); // WRONG!
>   *     if (err) {
>   *         handle the error...
>   *     }
>   * because this may pass a non-null err to bar().
> + *
> + * Likewise, do *not*
> + *     Error *err = NULL;
> + *     if (cond1) {
> + *         error_setg(err, ...);

s/err/&err

> + *     }
> + *     if (cond2) {
> + *         error_setg(err, ...); // WRONG!

ditto

With that fixed:

Reviewed-by: Greg Kurz <groug@kaod.org>

> + *     }
> + * because this may pass a non-null err to error_setg().
>   */
>  
>  #ifndef ERROR_H


Re: [PATCH 01/46] error: Improve examples in error.h's big comment
Posted by Markus Armbruster 5 years, 5 months ago
Greg Kurz <groug@kaod.org> writes:

> On Wed, 24 Jun 2020 18:42:59 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Show errp instead of &err where &err is actually unusual.  Add a
>> missing declaration.  Add a second error pileup example.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  include/qapi/error.h | 19 +++++++++++++++----
>>  1 file changed, 15 insertions(+), 4 deletions(-)
>> 
>> diff --git a/include/qapi/error.h b/include/qapi/error.h
>> index ad5b6e896d..1a5ea25e12 100644
>> --- a/include/qapi/error.h
>> +++ b/include/qapi/error.h
>> @@ -16,15 +16,15 @@
>>   * Error reporting system loosely patterned after Glib's GError.
>>   *
>>   * Create an error:
>> - *     error_setg(&err, "situation normal, all fouled up");
>> + *     error_setg(errp, "situation normal, all fouled up");
>>   *
>>   * Create an error and add additional explanation:
>> - *     error_setg(&err, "invalid quark");
>> - *     error_append_hint(&err, "Valid quarks are up, down, strange, "
>> + *     error_setg(errp, "invalid quark");
>> + *     error_append_hint(errp, "Valid quarks are up, down, strange, "
>>   *                       "charm, top, bottom.\n");
>>   *
>>   * Do *not* contract this to
>> - *     error_setg(&err, "invalid quark\n"
>> + *     error_setg(errp, "invalid quark\n" // WRONG!
>>   *                "Valid quarks are up, down, strange, charm, top, bottom.");
>>   *
>>   * Report an error to the current monitor if we have one, else stderr:
>> @@ -108,12 +108,23 @@
>>   *     }
>>   *
>>   * Do *not* "optimize" this to
>> + *     Error *err = NULL;
>>   *     foo(arg, &err);
>>   *     bar(arg, &err); // WRONG!
>>   *     if (err) {
>>   *         handle the error...
>>   *     }
>>   * because this may pass a non-null err to bar().
>> + *
>> + * Likewise, do *not*
>> + *     Error *err = NULL;
>> + *     if (cond1) {
>> + *         error_setg(err, ...);
>
> s/err/&err
>
>> + *     }
>> + *     if (cond2) {
>> + *         error_setg(err, ...); // WRONG!
>
> ditto

Good catch!

> With that fixed:
>
> Reviewed-by: Greg Kurz <groug@kaod.org>

Thanks!

>
>> + *     }
>> + * because this may pass a non-null err to error_setg().
>>   */
>>  
>>  #ifndef ERROR_H


Re: [PATCH 01/46] error: Improve examples in error.h's big comment
Posted by Vladimir Sementsov-Ogievskiy 5 years, 5 months ago
24.06.2020 19:42, Markus Armbruster wrote:
> Show errp instead of &err where &err is actually unusual.  Add a
> missing declaration.  Add a second error pileup example.
> 
> Signed-off-by: Markus Armbruster<armbru@redhat.com>

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

-- 
Best regards,
Vladimir

Re: [PATCH 01/46] error: Improve examples in error.h's big comment
Posted by Eric Blake 5 years, 5 months ago
On 6/24/20 11:42 AM, Markus Armbruster wrote:
> Show errp instead of &err where &err is actually unusual.  Add a
> missing declaration.  Add a second error pileup example.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   include/qapi/error.h | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
> 
Reviewed-by: Eric Blake <eblake@redhat.com>

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