[PATCH 2/6] Extend printfd() to collect extra information about fd

[Dmitry V. Levin]

On Thu, Mar 10, 2022 at 09:01:02PM +0900, Masatake YAMATO wrote:
[...]
> @@ -726,18 +735,36 @@ print_quoted_string_in_angle_brackets(const char *str)
>  }
>  
>  void
> -printfd_pid(struct tcb *tcp, pid_t pid, int fd)
> +printfd_pid_filling_info(struct tcb *tcp, pid_t pid, int fd, struct fd_info **fd_info)
>  {
>  	PRINT_VAL_D(fd);
>  
> +	bool should_print = false;
>  	char path[PATH_MAX + 1];
> -	if (pid > 0 && !number_set_array_is_empty(decode_fd_set, 0)
> +	if (pid > 0
> +	    && ((should_print = !number_set_array_is_empty(decode_fd_set, 0)) || fd_info)
>  	    && getfdpath_pid(pid, fd, path, sizeof(path)) >= 0) {
> +		/*
> +		 * If FD_INFO is given, FT_INFO must be filled.
> +		 *
> +		 * <A> If the information of fd will be printed (SHOULD_PRINT),
> +		 *     a helper function, printdev(), for printing also fills
> +		 *     FD_INFO as side effect.
> +		 * <B> If the information of fd will not be printed (!SHOULD_PRINT),
> +		 *     we must force to call the helper function for
> +		 *     filling FD_INFO. However, nothing should be
> +		 *     printed. Calling disable_tprint/enable_tprint
> +		 *     is for disabling the output stream.
> +		 */
> +		void *tprint_state = NULL;
> +		if (!should_print)
> +			tprint_state = disable_tprint();
> +
>  		if (is_number_in_set(DECODE_FD_SOCKET, decode_fd_set) &&
>  		    printsocket(tcp, fd, path))
>  			goto printed;
> -		if (is_number_in_set(DECODE_FD_DEV, decode_fd_set) &&
> -		    printdev(tcp, fd, path))
> +		if ((fd_info || is_number_in_set(DECODE_FD_DEV, decode_fd_set)) &&
> +		    printdev(tcp, fd, path, fd_info))
>  			goto printed;
>  		if (is_number_in_set(DECODE_FD_PIDFD, decode_fd_set) &&
>  		    printpidfd(pid, fd, path))
> @@ -745,6 +772,8 @@ printfd_pid(struct tcb *tcp, pid_t pid, int fd)
>  		if (is_number_in_set(DECODE_FD_PATH, decode_fd_set))
>  			print_quoted_string_in_angle_brackets(path);
>  printed:	;
> +		if (!should_print)
> +			enable_tprint(tprint_state);
>  	}

I'm sorry, but I don't like this disable_tprint/enable_tprint approach.
It's not just suboptimal because of wasted work in this function,
it also looks ugly.
If all you need from this function is printdev's side effect,
wouldn't it be better to split printdev instead?


-- 
ldv