Re: [PATCH 1/3] ACPI: fix incorrect counts returned by acpi_parse_entries_array()

From: Rafael J. Wysocki
Date: Fri Jul 01 2016 - 18:02:26 EST


On Fri, Jul 1, 2016 at 11:50 PM, Al Stone <ahs3@xxxxxxxxxx> wrote:
> On 07/01/2016 03:44 PM, Rafael J. Wysocki wrote:
>> On Fri, Jul 1, 2016 at 11:36 PM, Al Stone <ahs3@xxxxxxxxxx> wrote:
>>> On 07/01/2016 03:25 PM, Rafael J. Wysocki wrote:
>>>> On Fri, Jul 1, 2016 at 11:21 PM, Al Stone <ahs3@xxxxxxxxxx> wrote:
>>>>> The static function acpi_parse_entries_array() is provided an array of
>>>>> type struct acpi_subtable_proc that has a callback function and a count.
>>>>> The count should reflect how many times the callback has been successfully
>>>>> called. However, the current code only increments the 0th element of the
>>>>> array, regardless of the number of entries in the array, or which callback
>>>>> has been invoked. The fix is to use the index into the array, instead of
>>>>> a pointer to the beginning of the array.
>>>>
>>>> OK, so it would be good to say what the consequences of the problem are too.
>>>>
>>>
>>> Hrm. So replace the last sentence with something like:
>>>
>>> The fix is to use the index into the array, instead of
>>> a pointer to the beginning of the array, so that the count
>>> for each element in the array in incremented by the
>>> corresponding callback.
>>>
>>> That feels a little clunky but is it closer to what you were
>>> thinking?
>>
>> Well, not really.
>>
>> The code is arguably incorrect, but is there anything that does not
>> work as expected as a result? Any functional breakage? Any
>> misleading messages printed?
>>
>
> That's the odd thing; there is no breakage. Of any sort.
>
> But, no one relies on those values for anything at this point. I've got a
> couple of ideas I'm working on that are easier if it does work right, however.

That's information that should go into the changelog too.

"There are no functional consequences of the issue, but fixing it is
necessary for future work."

Or similar.