Re: [Qemu-devel] [PATCH 6/7] error: Revamp interface documentation

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 06/22/2015 01:26 PM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>  include/qapi/error.h | 176 
>> ++++++++++++++++++++++++++++++++++++---------------
>>  1 file changed, 126 insertions(+), 50 deletions(-)
>> 
>> diff --git a/include/qapi/error.h b/include/qapi/error.h
>> index 8c3a7dd..9466b09 100644
>> --- a/include/qapi/error.h
>> +++ b/include/qapi/error.h
>> @@ -2,13 +2,75 @@
>>   * QEMU Error Objects
>>   *
>>   * Copyright IBM, Corp. 2011
>> + * Copyright (C) 2011-2015 Red Hat, Inc.
>>   *
>>   * Authors:
>>   *  Anthony Liguori   <address@hidden>
>> + *  Markus Armbruster <address@hidden>,
>
> Trailing comma, but not intermediate, looks weird.

Fixing...

>>   *
>>   * This work is licensed under the terms of the GNU LGPL, version 2.  See
>>   * the COPYING.LIB file in the top-level directory.
>>   */
>> +
>> +/*
>> + * Error reporting system losely patterned after Glib's GError.
>
> s/losely/loosely/

Fixing...

>> + *
>> + * Receive an error and pass it on to the caller
>
> s/$/:/

Fixing...

>> + *
>> + * But when all you do with the error is passing it on, please use
>
> s/passing/pass/

Native speaker's advice accepted :)

>> +/*
>> + * Propagate error object (if any) from @local_err to @dst_errp.
>> + * If @local_err is NULL, do nothing (because there's nothing to
>> + * propagate).
>> + * Else, if @dst_errp is NULL, errors are being ignored.  Free the
>> + * error object.
>> + * Else, if @dst_errp, is &error_abort, print a suitable message and
>
> s/dst_errp,/dst_errp/

Fixing...

>> -/**
>> - * Free an error object.
>> +/*
>> + * Free @err.
>>   */
>>  void error_free(Error *err);
>
> Is error_free(NULL) safe? Worth documenting (because it affects
> paradigms used in cleanup labels).

Yes, it's safe, and yes, it's worth documenting, so I'll do it in v2.

> Overall, a definite improvement.

Thanks!