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

Dmitry V. Levin ldv at strace.io
Tue May 14 07:15:14 UTC 2024


On Tue, May 14, 2024 at 10:31:36AM +0530, Sahil wrote:
> Hi,
> 
> On Tuesday, May 14, 2024 3:07:04 AM GMT+5:30 Dmitry V. Levin wrote:
> > On Tue, May 14, 2024 at 12:27:16AM +0530, Sahil wrote:
> > > Hi,
> > > 
> > > On Monday, May 13, 2024 5:21:17 PM GMT+5:30 Dmitry V. Levin wrote:
> > > > On Fri, May 10, 2024 at 10:44:08AM +0530, Sahil Siddiq wrote:
> > > > > [...]
> > > > > @@ -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.
> > > 
> > > Sorry, I am not entirely clear on this. Even if a wrapper scan_fdinfo()
> > > is implemented around scan_fdinfo_array(), wouldn't the wrapper have
> > > different parameter types compared to the original scan_fdinfo() which
> > > has char *, size_t and a function pointer "scan_fdinfo_fn" as its
> > > parameters?
> > > 
> > > I haven't understood how this can be implemented without patching the
> > > call sites.
> > 
> > I mean something like this:
> > 
> > static inline bool
> > scan_fdinfo(pid_t pid, int fd, const char *search_pfx,
> >             size_t search_pfx_len, scan_fdinfo_fn fn, void *data)
> > {
> > 	struct scan_fdinfo lines[] = {
> > 		{
> > 			.search_pfx = search_pfx,
> > 			.search_pfx_len = search_pfx_len,
> > 			.fn = fn,
> > 			.data = data
> > 		}
> > 	};
> > 	return scan_fdinfo_array(pid, fd, lines, ARRAY_SIZE(lines));
> > }
> 
> I am confused about the situation in which the "scan_fdinfo_lines" array
> will have more than one element. I am not sure if I am missing something
> here. In the case of eventfd, for example, an array of strings (char**) will
> have to be passed to scan_fdinfo instead of a single string. In this case,
> the signature of the inline function "scan_fdinfo" will have to be changed,
> right? Similarly, an array of pointer functions will have to be passed to the
> inline function.

The idea is to continue using scan_fdinfo() for the case of single search
prefix, and use scan_fdinfo_array() for other cases.


-- 
ldv


More information about the Strace-devel mailing list