[RFC PATCH v3 2/5] Refactor scan_fdinfo and decode-fd functions

[Dmitry V. Levin]

On Fri, May 10, 2024 at 10:44:08AM +0530, Sahil Siddiq wrote:
> Create fdinfo struct. scan_fdinfo can use this
> struct to track which fd-specific fields are
> available in one pass of /proc/<pid>/fdinfo<fd>.
> 
> * src/util.c
>   (fdinfo): New struct.
>   (scan_fdinfo): Use this struct.
>   (get_finfo_for_dev): Likewise.
>   (pidfd_get_pid): Likewise.
>   (printsignalfd): Likewise.
> 
> Signed-off-by: Sahil Siddiq <icegambit91 at gmail.com>
> ---
> Changes v2 -> v3: New commit
> - src/util.c
>   (fdinfo): New struct.
>   (scan_fdinfo): Refactor to use this struct.
>   (get_finfo_for_dev): Likewise.
>   (pidfd_get_pid): Likewise.
>   (printsignalfd): Likewise.
> 
>  src/util.c | 69 +++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 53 insertions(+), 16 deletions(-)
> 
> diff --git a/src/util.c b/src/util.c
> index 4140cb718..f558d342c 100644
> --- a/src/util.c
> +++ b/src/util.c
> @@ -647,9 +647,16 @@ printsocket(struct tcb *tcp, int fd, const char *path)
>  
>  typedef bool (*scan_fdinfo_fn)(const char *value, void *data);
>  
> -static bool
> -scan_fdinfo(pid_t pid_of_fd, int fd, const char *search_pfx,
> -	    size_t search_pfx_len, scan_fdinfo_fn fn, void *data)
> +struct fdinfo {
> +	const char *search_pfx;
> +	size_t search_pfx_len;
> +	scan_fdinfo_fn fn;
> +	void *data;
> +};

Given that this new structure describes how to look for fdinfo rather than
fdinfo itself, let's give it a name that would match the purpose of the
structure.  I wouldn't mind calling it struct scan_fdinfo.

> +
> +static size_t
> +scan_fdinfo(pid_t pid_of_fd, int fd, struct fdinfo *fdinfo_search_array,
> +            size_t fdinfo_array_size)
>  {
>  	int proc_pid = 0;
>  	translate_pid(NULL, pid_of_fd, PT_TID, &proc_pid);
> @@ -665,21 +672,26 @@ scan_fdinfo(pid_t pid_of_fd, int fd, const char *search_pfx,
>  
>  	char *line = NULL;
>  	size_t sz = 0;
> -	bool result = false;
> +	size_t prefix_matches = 0;
>  
>  	while (getline(&line, &sz, f) > 0) {
> -		const char *value =
> -			str_strip_prefix_len(line, search_pfx, search_pfx_len);
> -		if (value != line && fn(value, data)) {
> -			result = true;
> -			break;
> +		for (size_t i = 0; i < fdinfo_array_size ; i++) {
> +			scan_fdinfo_fn fn = fdinfo_search_array[i].fn;
> +			void *data =  fdinfo_search_array[i].data;
> +			const char *value =
> +				str_strip_prefix_len(line, fdinfo_search_array[i].search_pfx,
> +				                     fdinfo_search_array[i].search_pfx_len);
> +			if (value != line && fn(value, data)) {
> +				prefix_matches += 1;

++prefix_matches or prefix_matches++ would look more natural.

> +				break;
> +			}
>  		}
>  	}
>  
>  	free(line);
>  	fclose(f);
>  
> -	return result;
> +	return prefix_matches;
>  }
>  
>  static bool
> @@ -734,8 +746,17 @@ get_finfo_for_dev(pid_t pid, int fd, const char *path, struct finfo *finfo)
>  		static const char prefix[] = "tty-index:\t";
>  
>  		finfo->dev.tty_index = -1;
> -		scan_fdinfo(pid, fd, prefix, sizeof(prefix) - 1,
> -			    set_tty_index, finfo);
> +
> +		struct fdinfo fdinfo_lines[] = {
> +			{
> +				.search_pfx = prefix,
> +				.search_pfx_len = sizeof(prefix) - 1,
> +				.fn = set_tty_index,
> +				.data = finfo
> +			}
> +		};
> +
> +		scan_fdinfo(pid, fd, fdinfo_lines, ARRAY_SIZE(fdinfo_lines));

There is an option to give the new edition of scan_fdinfo() a new name,
e.g. scan_fdinfo_array(), and create an inline wrapper scan_fdinfo()
around it, this way there won't be any need to patch its call sites.


-- 
ldv