On 11/20/2012 03:45 PM, Daniel P. Berrange wrote:
> On Tue, Nov 20, 2012 at 02:55:28PM +0100, Martin Kletzander wrote:
>> The error "... but the cause is unknown" appeared for XMLs similar to
>> this:
>>
>> <disk type='file' device='cdrom'>
>> <driver name='qemu' type='raw'/>
>> <source file='/dev/zero'/>
>> <target dev='sr0'/>
>> </disk>
>>
>> Notice unsupported disk type (for the driver), but also no address
>> specified. The first part is not a problem and we should not abort
>> immediately because of that, but the combination with the address
>> unknown was causing an unspecified error.
>> ---
>> src/util/util.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/util/util.c b/src/util/util.c
>> index 75b18c1..d5b2c97 100644
>> --- a/src/util/util.c
>> +++ b/src/util/util.c
>> @@ -2189,8 +2189,12 @@ int virDiskNameToIndex(const char *name) {
>> }
>> }
>>
>> - if (!ptr)
>> + if (!ptr) {
>> + virReportError(VIR_ERR_XML_ERROR,
>> + _("Unknown disk name '%s' and no address specified"),
>> + name);
>> return -1;
>> + }
>>
>> for (i = 0; *ptr; i++) {
>> idx = (idx + (i < 1 ? 0 : 1)) * 26;
>
> IMHO, you should really be adding an ATTRIBUTE_NONNULL annotation and
> then fixing the callers not to invoke it if the disk name they have
> is Null.
>
The pointer is not the function attribute, but ...
> Adding virReportError here is wrong, because a number of callers will
> already report errors.
>
... you're right as Peter, v2 is on it's way.
Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list