Re: [PATCH i-g-t v4 1/2] lib/igt_kmemleak: library to interact with kmemleak

[Peter Senna Tschudin <peter.senna@linux.intel.com>]

Hi Kamil,

Thank you, please see my comments.

On 31.01.2025 13:38, Kamil Konieczny wrote:
> Hi Peter,
> On 2025-01-28 at 16:15:36 +0100, Peter Senna Tschudin wrote:
> 
> sorry one more nit, found by checkpatch, see below.
> 
>> Adds a simple library for interacting with kmemleak and add
>> unit testing for the library. There are two modes intended to
>> integrate with igt_runner:
>> - once: collect kmemleaks after all test completed
>> - each: collect kmemleaks after eachb test completes
>>
>> To use the library include "igt_kmemleak.h", call
>> igt_kmemleak_init(NULL) to check if kmemleak is enabled and finally
>> call igt_kmemleak() to collect kmemleaks. igt_kmemleak() expect the
>> following arguments:
>>   - const char *last_test: Name of the last lest or NULL
>>   - int resdirfd: file descriptor of the results directory
>>   - bool kmemleak_each: Are we scanning once or scanning after
>>     each test?
>>   - bool sync: sync after each write?
>>
>> CC: ryszard.knop@intel.com
>> CC: lucas.demarchi@intel.com
>> CC: katarzyna.piecielska@intel.com
>> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
>> Signed-off-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
>> ---
>>  lib/igt_kmemleak.c       | 274 +++++++++++++++++++++++++++++++++++++++
>>  lib/igt_kmemleak.h       |  16 +++
>>  lib/meson.build          |   1 +
>>  lib/tests/igt_kmemleak.c | 267 ++++++++++++++++++++++++++++++++++++++
>>  lib/tests/meson.build    |   1 +
>>  5 files changed, 559 insertions(+)
>>  create mode 100644 lib/igt_kmemleak.c
>>  create mode 100644 lib/igt_kmemleak.h
>>  create mode 100644 lib/tests/igt_kmemleak.c
>>
> 
> ...cut...
> 
>> +
>> +static const char *igt_kmemleak_unit_testing_resdir = "/tmp";
>> +
>> +igt_main
>> +{
>> +	char unit_testing_kmemleak_filepath[256] = "/tmp/igt_kmemleak_test_XXXXXX";
>> +	int written_bytes;
>> +	int resdirfd;
>> +	int fd;
>> +
>> +	igt_fixture {
>> +		/* resdirfd is used by igt_kmemleak() to store results */
>> +		igt_assert(resdirfd = open(igt_kmemleak_unit_testing_resdir,
>> +					   O_DIRECTORY | O_RDONLY));
>> +
>> +		/* Try to delete results file in case of leftovers,
>> +		 * ignoring errors as the file may not exist
>> +		 */
>> +		unlinkat(resdirfd, KMEMLEAKRESFILENAME, 0);
>> +
>> +		/* Creating a fake kmemleak file for unit testing */
>> +		fd = mkstemp(unit_testing_kmemleak_filepath);
>> +		igt_assert(fd >= 0);
>> +
>> +		written_bytes = write(fd, kmemleak_file_example,
>> +				strlen(kmemleak_file_example));
> 
> Align here or just write this in one line:
> 		written_bytes = write(fd, kmemleak_file_example, strlen(kmemleak_file_example));
> or
> 		written_bytes = write(fd, kmemleak_file_example,
> 				      strlen(kmemleak_file_example));

Good catch, curious why my checkpatch did not catch this one:

---
psennats@friendship7-home:~/UPSTREAM/igt-gpu-tools$ ../linux/scripts/checkpatch.pl 0001-lib-igt_kmemleak-library-to-interact-with-kmemleak.patch
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#39:
new file mode 100644

WARNING: quoted string split across lines
#394: FILE: lib/tests/igt_kmemleak.c:37:
+"   [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
+"unreferenced object 0xffff888102a2ed18 (size 80):\n"

WARNING: quoted string split across lines
#416: FILE: lib/tests/igt_kmemleak.c:59:
+"   [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
+"unreferenced object 0xffff888102a2ea58 (size 80):\n"

WARNING: quoted string split across lines
#438: FILE: lib/tests/igt_kmemleak.c:81:
+"   [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
+"unreferenced object 0xffff888102a2e428 (size 80):\n"

WARNING: quoted string split across lines
#460: FILE: lib/tests/igt_kmemleak.c:103:
+"   [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
+"unreferenced object 0xffff888102a2e008 (size 80):\n"

WARNING: quoted string split across lines
#482: FILE: lib/tests/igt_kmemleak.c:125:
+"   [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
+"unreferenced object 0xffff888102a2e2c8 (size 80):\n"

WARNING: quoted string split across lines
#504: FILE: lib/tests/igt_kmemleak.c:147:
+"   [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
+"unreferenced object 0xffff888102a2e378 (size 80):\n"

WARNING: quoted string split across lines
#526: FILE: lib/tests/igt_kmemleak.c:169:
+"   [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
+"unreferenced object 0xffff888102a2e798 (size 80):\n"

WARNING: quoted string split across lines
#548: FILE: lib/tests/igt_kmemleak.c:191:
+"   [<ffffffff824ca53b>] kernel_init+0x1b/0x170"
+"unreferenced object 0xffff888102a2e0b8 (size 80):\n"

total: 0 errors, 9 warnings, 571 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

0001-lib-igt_kmemleak-library-to-interact-with-kmemleak.patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.
---



> 
> Regards,
> Kamil
> 
> ...cut...