qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH trivial] acpi: actually require either data= or


From: Michael Tokarev
Subject: Re: [Qemu-devel] [PATCH trivial] acpi: actually require either data= or file= for -acpitable
Date: Mon, 03 Jun 2013 16:42:09 +0400
User-agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:10.0.12) Gecko/20130116 Icedove/10.0.12

03.06.2013 16:34, Eric Blake wrote:
> On 06/03/2013 03:20 AM, Michael Tokarev wrote:
>> Initially the code ensured that we have exactly one of data= or file= option 
>> for -acpitable.  But after some transformations, the condition becomes
>> 
>> if (has_data == has_file) { error }
>> 
>> to mean, probably, that both should not be set at the same time.  But this 
>> condition does not cover the case when neither is set, and we generate bogus 
>> acpi table in this case.
>> 
>> Instead, check if sum of the two is exactly 1.
>> 
>> Signed-off-by: Michael Tokarev <address@hidden> --- hw/acpi/core.c |    2 +- 
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/hw/acpi/core.c b/hw/acpi/core.c index 42eeace..2815d8c 100644 
>> --- a/hw/acpi/core.c +++ b/hw/acpi/core.c @@ -249,7 +249,7 @@ void 
>> acpi_table_add(const QemuOpts *opts, Error **errp) if (err) { goto out; } -  
>>   if (hdrs->has_file == hdrs->has_data) {
> 
> NACK.
> 
> hdrs->has_file and hdrs->has_data are both bool.

Yup.

> Pre-patch, the table of possible combinations is:
> 
> false == false => error message, zero given false == true => okay, exactly 
> one given true == false => okay, exactly one given true == true => error 
> message, two given
> 
> which is already what you want.

Ok, you're right.  Thank you for spotting this!

This function has another error -- if the file specified
(either for data= or file=) can't be read, it happily
continues instead of erroring out.  _That_ is the bug
I tried to hunt but catched something else entirely ;)

Will send a real fix later today... :)

Thanks,

/mjt



reply via email to

[Prev in Thread] Current Thread [Next in Thread]