Re: [LTP] [PATCH v3] ioctl11.c: New case check PROCMAP_QUERY ioctl() errnos

[Petr Vorel <pvorel@suse.cz>]

Hi Wei, Andrea,

> +/*\
> + * [Description]
nit: we stopped using this. Could it be removed before merge?

> + *
> + * Test PROCMAP_QUERY ioctl() errnos:
> + *
> + * - EINVAL if q->size is too small
> + * - E2BIG if q->size is larger than page size
> + * - EINVAL on invalid q->flags
> + * - EINVAL if only one of q->vma_name_size and q->vma_name_addr is set
> + * - EINVAL if only one of q->build_id_size and q->build_id_addr is set
> + * - ENAMETOOLONG if build_id_size or name_buf_size is too small
> + * - ESRCH if attempt to get memory map of a process that did exit and was waited for
> + *
> + *   [1] https://kernelnewbies.org/Linux_6.11#Memory_management
> + *   [2] https://kernelnewbies.org/Linux_6.11#Binary_interface_for_.2Fproc.2F.3Cpid.3E.2Fmaps
> + *   [3] https://lore.kernel.org/all/20240627170900.1672542-1-andrii@kernel.org/
> + *       ed5d583a88a9 ("fs/procfs: implement efficient VMA querying API for /proc/<pid>/maps")

nit: [1] and other numbers will look ugly in generated doc:

[1] https://kernelnewbies.org/Linux_6.11#Memory_management [2] https://kernelnewbies.org/Linux_6.11#Binary_interface_for_.2Fproc.2F.3Cpid.3E.2Fmaps [3] https://lore.kernel.org/all/20240627170900.1672542-1-andrii@kernel.org/

=> inlined and [1] [2] [3] does not make sense when they refer to nothing.

Instead of
> + *   [1] https://kernelnewbies.org/Linux_6.11#Memory_management
> + *   [2] https://kernelnewbies.org/Linux_6.11#Binary_interface_for_.2Fproc.2F.3Cpid.3E.2Fmaps
> + *   [3] https://lore.kernel.org/all/20240627170900.1672542-1-andrii@kernel.org/
> + *       ed5d583a88a9 ("fs/procfs: implement efficient VMA querying API for /proc/<pid>/maps")

I suggest to change before merge to something like:

PROCMAP_QUERY was introduced in 6.11.

 * * https://kernelnewbies.org/Linux_6.11#Binary_interface_for_.2Fproc.2F.3Cpid.3E.2Fmaps
 * * ed5d583a88a9 ("fs/procfs: implement efficient VMA querying API for /proc/<pid>/maps")

Reviewed-by: Petr Vorel <pvorel@suse.cz>

...
> +static void fork_child_setup(void)
> +{
> +	int pid = SAFE_FORK();
> +
> +	if (pid == 0) {
> +		TST_CHECKPOINT_WAKE(0);
> +		TST_CHECKPOINT_WAIT(1);
> +		exit(0);
> +	}
> +
> +	TST_CHECKPOINT_WAIT(0);
> +
> +	char proc_path[PATH_MAX];
> +
> +	snprintf(proc_path, PATH_MAX, "/proc/%d/maps", pid);
> +	fd = SAFE_OPEN(proc_path, O_RDONLY);
> +
> +	TST_CHECKPOINT_WAKE(1);

Using of the checkpoints LGTM, but hopefully somebody else recheck it.

Kind regards,
Petr

> +	SAFE_WAITPID(pid, NULL, 0);
> +}

Kind regards,
Petr

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