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

[Laszlo Ersek]

On 06/03/13 14:42, Michael Tokarev wrote:
> 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... :)

Please CC me; IIRC I sought to test this code reasonably deeply when I
wrote / transformed it. I'm curious.

Thanks,
Laszlo