Re: [PATCH v2 1/6] selftests/resctrl: Introduced linked list management for IMC counters

[wuyifan <wuyifan50@huawei.com>]

Hi Reinette

On 5/8/2026 12:13 AM, Reinette Chatre wrote:
>> Calling cleanup_read_mem_bw_imc() in the error exit path may be intended
>> to prevent resource leaks. However, this results in the function being called
>> repeatedly in both the error exit branch and test_cleanup().
> You are correct and calling it repeatedly is ok. When cleanup_read_mem_bw_imc() is
> called from test_cleanup() after a failure in parse_imc_read_bw_events() then it
> will find that the list is empty and just be a no-op. This is safe.
>
>> Is there any specific intention behind calling it in parse_imc_read_bw_events()?
> The motivation behind calling it in parse_imc_read_bw_events() is to not leave this
> memory allocated when this function fails. A function having a single responsibility
> is easier to use and maintain since a caller does not need to take into account that
> when the function fails it also needs to have additional responsibility to clean up
> the state left behind by it.
>
> There may be some patterns where caller needs to clean up after a failure but that is
> usually done in an obvious way where the caller_immediately_ does the cleanup on failure
> but here this dependency is well hidden in this implementation with test_cleanup() being
> called so far from parse_imc_read_bw_events(). This hidden dependency makes this code
> difficult to use and maintain.
>
>> Or should the cleanup be uniformly handled in test_cleanup()?
> Handling it only in test_cleanup() may work in current execution flow but if the code is
> ever re-factored this would result in a memory leak. It is not custom that callers need
> to clean up state when a function fails and since this allocation is buried deep within the
> execution flow I see this as a latent bug just waiting to be triggered.

Thank you for the detailed explanation. I understand the rationale now.

I will explicitly add cleanup calls in the error exit path in the next 
version.

Best regards,

Yifan