[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