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

[Kamil Konieczny]

Hi Peter,
On 2025-02-03 at 10:14:19 +0100, Peter Senna Tschudin wrote:
> 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 at intel.com
> >> CC: lucas.demarchi at intel.com
> >> CC: katarzyna.piecielska at intel.com
> >> Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> >> Signed-off-by: Peter Senna Tschudin <peter.senna at 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 at 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"
> 

You could ignore this, checkpatch is not perfect.

Regards,
Kamil

[...]
> 
> 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...
>