[libvirt] [PATCH 1/2] xml: Print file name on parse error

[Peter Krempa]

On 08/16/2011 04:37 PM, Eric Blake wrote:
> On 08/16/2011 06:49 AM, Peter Krempa wrote:
>> This patch prints file name of currently parsed document
>> if an parse error occurs. While parsing XML documments from
>> string, user may specify NULL as URL and the original error message
>> (not containing filename information) is printed.
>>
>> Fixes BZ #726771
>> ---
>>   src/util/xml.c |   16 ++++++++++++----
>>   1 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/util/xml.c b/src/util/xml.c
>> index 05317ea..0e93d48 100644
>> --- a/src/util/xml.c
>> +++ b/src/util/xml.c
>> @@ -647,10 +647,18 @@ catchXMLError(void *ctx, const char *msg 
>> ATTRIBUTE_UNUSED, ...)
>>           if (virGetLastError() == NULL&&
>>               ctxt->lastError.level == XML_ERR_FATAL&&
>>               ctxt->lastError.message != NULL) {
>> -            virGenericReportError(domcode, VIR_ERR_XML_DETAIL,
>> -                                  _("at line %d: %s"),
>> -                                  ctxt->lastError.line,
>> -                                  ctxt->lastError.message);
>> +            if (ctxt->lastError.file) {
>> +                virGenericReportError(domcode, VIR_ERR_XML_DETAIL,
>> +                                      _("in file \'%s\'at line %d: 
>> %s"),
>
> Missing a space, and \' is redundant inside "" (you can get away with 
> just ').  At a higher level, when you have both a file and a line, it 
> is conventional to write this in a shorter form:
>
> "%s:%d: %s", file, line, message
Yes, that definitely looks better. I somehow tried to stick with the 
previous message and didn't think much about it.
>
>> +                                      ctxt->lastError.file,
>> +                                      ctxt->lastError.line,
>> +                                      ctxt->lastError.message);
>> +            } else {
>> +                virGenericReportError(domcode, VIR_ERR_XML_DETAIL,
>> +                                      _("at line %d: %s"),
>
> It is only here, where you don't have a file, that including the extra 
> text "at line %d:" is appropriate (besides the fact that it preserves 
> the original error spelling).
>
Message fixed in v2.

Peter