Re: [PATCH v3 1/4] tools/lib/mm: add shared file helpers

[Sarthak Sharma <sarthak.sharma@arm.com>]

Hi Mike!

On 5/24/26 10:36 PM, Mike Rapoport wrote:
> On Thu, 21 May 2026 16:47:58 +0530, Sarthak Sharma <sarthak.sharma@arm.com> wrote:
> 
> Hi Sarthak,
> 
>>
>> diff --git a/tools/lib/mm/file_utils.c b/tools/lib/mm/file_utils.c
>> new file mode 100644
>> index 000000000000..0f9322f2cf41
>> --- /dev/null
>> +++ b/tools/lib/mm/file_utils.c
>> @@ -0,0 +1,83 @@
>> [ ... skip 48 lines ... ]
>> +	saved_errno = errno;
>> +	close(fd);
>> +	errno = saved_errno;
>> +	if (numwritten < 0) {
>> +		fprintf(stderr, "%s write(%.*s) failed: %s\n",
>> +			path, (int)(buflen - 1), buf, strerror(errno));
> 
> This would break TAP formatting for selftests.

Yes, thanks for pointing it out.

> 
>> +		exit(EXIT_FAILURE);
> 
> and while EXIT_FAILURE == KSFT_FAIL I'm not sure it's robust enough.

I used EXIT_FAILURE here because the helper is moving out of selftests
and should not include kselftest.h anymore. The helper already
terminated the process on these paths, so I tried to preserve that
behavior while removing the ksft dependency.

We can change this to return errors instead of calling exit() and update
the selftest callers to report failures through the ksft_* helpers. I
agree this is cleaner, but it would grow the series a bit.

If you feel strongly, I can include these changes in v4. Otherwise I
feel we can handle it separately later to avoid growing this series.

> 
>>
>> diff --git a/tools/testing/selftests/mm/hugepage_settings.c b/tools/testing/selftests/mm/hugepage_settings.c
>> index 2eab2110ac6a..5e947abb7425 100644
>> --- a/tools/testing/selftests/mm/hugepage_settings.c
>> +++ b/tools/testing/selftests/mm/hugepage_settings.c
>> @@ -8,8 +8,9 @@
>>  #include <stdlib.h>
>>  #include <string.h>
>>  #include <unistd.h>
>> +#include <mm/file_utils.h>
>>  
>> -#include "vm_util.h"
> 
> I think it would be fine to include file_utils.h in vm_utils.h and avoid
> further churn.

Okay, I'll change this.