Re: [LTP] [PATCH v6 2/2] coredump02: Verify ELF structure and notes

[Li Wang <li.wang@linux.dev>]

Hi Andrea,

Overall the test methodology looks good, with just a few minor
code cleanup points below.

> +static void set_pattern(const char *fmt, ...)
> +{
> +	char pattern[PATTERN_MAX];
> +	char readback[PATTERN_MAX];
> +	va_list va;
> +	int len;
> +
> +	va_start(va, fmt);
> +	len = vsnprintf(pattern, sizeof(pattern), fmt, va);
> +	va_end(va);
> +
> +	if (len >= PATTERN_MAX)
> +		tst_brk(TCONF, "core_pattern does not fit into %i bytes", PATTERN_MAX - 1);
> +
> +	SAFE_FILE_PRINTF(PATH_KERN_CORE_PATTERN, "%s", pattern);
> +	SAFE_FILE_LINES_SCANF(PATH_KERN_CORE_PATTERN, "%127[^\n]", readback);
> +
> +	if (strcmp(pattern, readback))
> +		tst_brk(TBROK, "core_pattern readback mismatch: wrote '%s', read '%s'",
> +			pattern, readback);
> +
> +	tst_res(TINFO, "core_pattern is '%s'", pattern);
> +}
> +
> +static pid_t crash_child(void)
> +{
> +	int status;
> +	pid_t pid;
> +
> +	pid = SAFE_FORK();
> +	if (!pid)
> +		abort();
> +
> +	SAFE_WAITPID(pid, &status, 0);
> +
> +	if (!WIFSIGNALED(status) || !WCOREDUMP(status))
> +		tst_brk(TFAIL, "Child did not dump core");
> +
> +	return pid;
> +}

Splitting the core file parsing into a separate test is fine.
For the duplicated code, there are two approaches - I'm okay
with either.

1. create a seperate coredump_common.h

	#cat coredump_common.h
	...

	#ifndef COREDUMP_COMMON_H
	#define COREDUMP_COMMON_H
	
	#include <elf.h>
	#include <sys/wait.h>
	
	#include "tst_test.h"
	#include "lapi/prctl.h"
	
	#define PATTERN_MAX 128
	
	static char cwd[PATH_MAX];
	
	static void set_pattern(const char *fmt, ...)
	{
		// ...
	}
	
	static pid_t crash_child(void)
	{
		// ...
	}
	
	static void coredump_setup(void)
	{
		struct rlimit rl = {RLIM_INFINITY, RLIM_INFINITY};
	
		SAFE_SETRLIMIT(RLIMIT_CORE, &rl);
		SAFE_PRCTL(PR_SET_DUMPABLE, 1, 0, 0, 0);
		SAFE_GETCWD(cwd, sizeof(cwd));
	}
	
	#endif /* COREDUMP_COMMON_H */


2. Merge coredump02 method into coredump01 with use tcnt=3.

	static void run(unsigned int n)
	{
		switch (n) {
		case 0:
			verify_file_pattern_specifiers();
			break;
		case 1:
			verify_file_pattern_content();
			break;
		case 2:
			verify_pipe_pattern();
			break;
		}
	}
	
	static struct tst_test test = {
		.test = run,
		.tcnt = 3,
		// ...
	};


-- 
Regards,
Li Wang

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