Re: [LTP] [PATCH v1 2/2] ptrace06: Refactor the test using new LTP API

[Petr Vorel <pvorel@suse.cz>]

Hi Wei,

note ptrace06 was not even been compiled. I tested that even in old API it
worked, thus I re-enable it in patch

https://patchwork.ozlabs.org/project/ltp/patch/20231128091524.340808-3-pvorel@suse.cz/
(part of https://patchwork.ozlabs.org/project/ltp/list/?series=384172&state=*)

> +++ b/testcases/kernel/syscalls/ptrace/ptrace06.c
> @@ -1,32 +1,31 @@
> +// SPDX-License-Identifier: GPL-2.0-only
Again:
// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> + * Copyright (c) 2008 Analog Devices Inc.
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
>   * check out-of-bound/unaligned addresses given to
Without missing blank line here the list will not be working.
>   *  - {PEEK,POKE}{DATA,TEXT,USER}
>   *  - {GET,SET}{,FG}REGS
>   *  - {GET,SET}SIGINFO
>   *
Why this blank line above?

> - * Copyright (c) 2008 Analog Devices Inc.
> - *
> - * Licensed under the GPL-2 or later
... because this:
>   */

>  #define _GNU_SOURCE
This might not be needed (needs to be verified in CI if also the oldest distros
does not need it).

>  /* this should be sizeof(struct user), but that info is only found
>   * in the kernel asm/user.h which is not exported to userspace.
>   */
> +
>  #if defined(__i386__)
>  #define SIZEOF_USER 284
>  #elif defined(__x86_64__)
> @@ -35,168 +34,213 @@
>  #define SIZEOF_USER 0x1000	/* just pick a big number */
>  #endif

I wonder if this SIZEOF_USER is valid. I haven't found what value they mean
(they talk about arch/*/include/asm/user*.h in kernel).

> -char *TCID = "ptrace06";
> -
>  struct test_case_t {
>  	int request;
>  	long addr;
>  	long data;
>  } test_cases[] = {
>  	{
> +	PTRACE_PEEKDATA, .addr = 0}, {
> +	PTRACE_PEEKDATA, .addr = 1}, {
IMHO This is ugly formatting, brackets shold be always on the same place.
.e.g 
{ PTRACE_PEEKDATA, .addr = 0},
{ PTRACE_PEEKDATA, .addr = 1},


> +	PTRACE_PEEKDATA, .addr = 2}, {
> +	PTRACE_PEEKDATA, .addr = 3}, {
> +	PTRACE_PEEKDATA, .addr = -1}, {
> +	PTRACE_PEEKDATA, .addr = -2}, {
...
> +static void child(void)
> +{
> +	SAFE_PTRACE(PTRACE_TRACEME, 0, NULL, NULL);
> +	execl("/bin/echo", "/bin/echo", NULL);
This will not work for AOSP (Android). Maybe adding ptrace06_child.c with very
simple code (printf or tst_res(TINFO) something, use TST_NO_DEFAULT_MAIN) would
be better.

> +	exit(0);
> +}

...

Kind regards,
Petr

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