Re: [PATCH 3/3] ACPI: fix acpi_parse_entries_array() so it reports overflow correctly

From: Rafael J. Wysocki
Date: Fri Jul 01 2016 - 17:54:30 EST


On Fri, Jul 1, 2016 at 11:44 PM, Al Stone <ahs3@xxxxxxxxxx> wrote:
> On 07/01/2016 03:40 PM, Rafael J. Wysocki wrote:
>> On Fri, Jul 1, 2016 at 11:21 PM, Al Stone <ahs3@xxxxxxxxxx> wrote:
>>> The function acpi_parse_entries_array() has a limiting parameter,
>>> max_entries, which tells the function to stop looking at subtables
>>> once that limit has been reached. Further, if the limit is reached,
>>> it is reported. However, the logic is incorrect in that the loop
>>> to examine all subtables will always stop when exactly max_entries
>>> have been found, regardless of whether or not there are still subtables
>>> to examine, and it will always report that zero subtables have been
>>> ignored. This change allows the loop to continue to look at all
>>> subtables and count all the ones of interest; if we have already
>>> reached the number of max_entries, though, we will not invoke the
>>> callback functions. If the max_entries limit has been exceeded,
>>> report on that, as before, but more accurately, listing how many
>>> subtables of interest there are in total (as was meant), and how
>>> many entries each subtable type occupied.
>>
>> The problem appears to be that, if max_entries has been reached, it
>> prints "ignored 0", although it should count all of the entries in
>> that case too in principle. Do I think correctly?
>>
>
> Exactly. That's how I interpreted the comments. And it fit what I
> needed it to do if the comments were correct.
>
> Of course, it could be the code was correct and the comments were
> wrong :). I preferred not to think that.

I guess whoever implemented this function thought that the overhead
for counting stuff was not useful in case max_entries had been
reached. I'm not really sure I disagree with that. :-)

I agree that printing "ignored 0" in that case is misleading, but the
fix might be to simply avoid printing how many entries have been
ignored then. Maybe it will suffice to print how many entries have
been found and what the limit was?