Eric Blake <ebl...@redhat.com> writes: > On 12/17/2015 11:04 AM, Markus Armbruster wrote: > >>>> + * Create an error and add additional explanation: >>>> + * error_setg(&err, "invalid quark"); >>>> + * error_append_hint(&err, "Valid quarks are up, down, strange, " >>>> + * "charm, top, bottom\n"); >>> >>> Do we want to allow/encourage a trailing dot in the hint? I'm fine if >>> we don't - but once we document its usage, we should probably then be >>> consistent with what we document; qemu-option.c used a trailing dot, >>> qdev-monitor.c did not. >> >> I'll fix this example. >> >>>> + * >>>> + * Do *not* contract this to >>>> + * error_setg(&err, "invalid quark\n" >>>> + * "Valid quarks are up, down, strange, charm, top, bottom"); > > Of course, if you put a trailing dot above, you'd also want it here in > the 'don't contract' section.
Yes. >>>> @@ -128,6 +138,7 @@ ErrorClass error_get_class(const Error *err); >>>> * If @errp is anything else, *@errp must be NULL. >>>> * The new error's class is ERROR_CLASS_GENERIC_ERROR, and its >>>> * human-readable error message is made from printf-style @fmt, ... >>>> + * The resulting message should not contain newlines. >>> >>> Should we also discourage trailing punctuation? >> >> Yes. How to best phrase it? > > Maybe: > > The resulting message should be a single phrase, with no newline or > trailing punctuation. What about ending the message with an exclamation mark? >>> Should we also mention no need for leading 'error: ' prefix? >> >> Unlike the other anti-patterns, this one doesn't seem frequent in new >> code. > > Fair enough. > > >>>> /** >>>> * Append a printf-style human-readable explanation to an existing error. >>>> - * May be called multiple times, and safe if @errp is NULL. >>>> + * @errp may be NULL, but neither &error_fatal nor &error_abort. >>> >>> As a native speaker, 'not' sounds better than 'neither' in that >>> sentence. But I think both choices are grammatically correct. >> >> You mean "not &error_abort or &error_abort"? > > Yes, that works: > > @errp may be NULL, but not &error_fatal or &error_abort.
