Re: [LTP] [PATCH v4 1/2] tst_memutils.c: Add tst_print_meminfo function

[Petr Vorel <pvorel@suse.cz>]

Hi Wei,

> +void safe_print_file(const char *file, const int lineno, char *path)
> +{
> +	FILE *pfile;
> +	char line[PATH_MAX];
> +
> +	pfile = safe_fopen(file, lineno, NULL, path, "r");
> +
I'd expect here TINFO message:

tst_res(TINFO, "=== %s ===", path);

> +	while (fgets(line, sizeof(line), pfile))
> +		tst_resm_(file, lineno, TINFO, "%s", line);
This will not work, that's legacy API.

Please next time pay attention with warnings:

tst_safe_macros.c: In function ‘safe_print_file’:
tst_safe_macros.c:622:17: warning: implicit declaration of function ‘tst_resm_’; did you mean ‘tst_res_’? [-Wimplicit-function-declaration]
  622 |                 tst_resm_(file, lineno, TINFO, "%s", line);
      |                 ^~~~~~~~~
      |                 tst_res_


Also, if you push to github, you see this is fatal for CI, where we compile with
-Werror=implicit-function-declaration.

Please next time push to github first, so that you save reviewer time.

Also, when you run the test, the output contains extra new line:

tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_test.c:1690: TINFO: LTP version: 20230929-201-ge32735252
tst_test.c:1574: TINFO: Timeout per run is 0h 10m 30s
tst_safe_macros.c:619: TINFO: === /proc/meminfo ===
swapping01.c:88: TINFO: MemTotal:        7700224 kB

swapping01.c:88: TINFO: MemFree:         4058996 kB

swapping01.c:88: TINFO: MemAvailable:    4172736 kB
...

We could remove it, but actually printing it via fprintf is actually better:

tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_test.c:1690: TINFO: LTP version: 20230929-201-ge32735252
tst_test.c:1574: TINFO: Timeout per run is 0h 10m 30s
tst_safe_macros.c:619: TINFO: === /proc/meminfo ===
MemTotal:        1467180 kB
MemFree:          820040 kB
MemAvailable:    1143320 kB
...

Anyway, fixed and merged this patch.

Kind regards,
Petr

> +
> +	safe_fclose(file, lineno, NULL, pfile);
> +}

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp