Hi,
On 6/16/22 13:27, Andy Shevchenko wrote:
> On Thu, Jun 16, 2022 at 11:13 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>> On 6/15/22 23:31, Andy Shevchenko wrote:
>>> On Wed, Jun 15, 2022 at 9:57 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> ...
>
>>>> - char path[ACPI_PATH_SEGMENT_LENGTH + 2];
>>>> + char path[ACPI_PATH_SEGMENT_LENGTH * 2 + 2];
>>>
>>> Strictly speaking this should be, IIUC,
>>>
>>> 1 + ACPI_PATH_SEGMENT_LENGTH + ACPI_NAMESEG_SIZE + 1
>>>
>>> \\ + path segment length (with a separator) + name + \0
>>>
>>> That said, it seems the original code adds 1 unneeded byte.
>>
>> Right I already knew I needed the " * 2" when writing the
>> original code, so I decided to do things this way for KISS
>> reasons.
>
> Yeah, but then you don't need + 2, it should be +1.
> I believe the initial use of ACPI_PATH_SEGMENT_LENGTH vs.
> ACPI_NAMESEG_SIZE is a bit misleading.
You are technicaly correct, but the code is IMHO just
easier to read like this:
char path[ACPI_PATH_SEGMENT_LENGTH * 2 + 2];
Then like this:
char path[1 + ACPI_NAMESEG_SIZE + ACPI_PATH_SEGMENT_LENGT + 1];
And also to be more technically correct it would need to be:
char path[1 + strlen("_SB") + ACPI_PATH_SEGMENT_LENGT + 1];
Since "_SB" only uses 3 of the 4 bytes in ACPI_NAMESEG_SIZE,
so that would save us not 1 but 2 bytes on the stack.
But those 2 stack bytes are really not worth the worse
readability of these options IMHO.
Regards,
Hans
>
>>> Perhaps a comment in the code?
>>
>> I've added a comment to my local version now.
>>
>> After this thread has had some more time to gather feedback
>> I will turn the ACPICA patches into a acpica github pull-req
>> with the comment added.
>>
>> (or if necessary rework the entire series)
>
>
