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 <m...@tls.msk.ru> --- 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
