Re: [PATCH v4 09/19] selftests/resctrl: Convert span to size_t

[Ilpo Järvinen]

On Thu, 13 Jul 2023, Reinette Chatre wrote:

> Hi Ilpo,
> 
> On 7/13/2023 6:19 AM, Ilpo Järvinen wrote:
> 
> ...
> 
> > @@ -188,10 +188,10 @@ fill_cache(unsigned long long buf_size, int malloc_and_init, int memflush,
> >  	return 0;
> >  }
> >  
> > -int run_fill_buf(unsigned long span, int malloc_and_init_memory,
> > -		 int memflush, int op, char *resctrl_val)
> > +int run_fill_buf(size_t span, int malloc_and_init_memory, int memflush, int op,
> > +		 char *resctrl_val)
> >  {
> > -	unsigned long long cache_size = span;
> > +	size_t cache_size = span;
> >  	int ret;
> >  
> >  	ret = fill_cache(cache_size, malloc_and_init_memory, memflush, op,
> 
> Any idea what the purpose being run_fill_buf() is? From what I can tell it is
> an unnecessary level of indirection.

You already mentioned it, fill_cache() could be included into 
run_fill_buf() but it's not part of this series.

> > diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
> > index f622245adafe..8be5b745226d 100644
> > --- a/tools/testing/selftests/resctrl/resctrlfs.c
> > +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> > @@ -298,7 +298,7 @@ int taskset_benchmark(pid_t bm_pid, int cpu_no)
> >  void run_benchmark(int signum, siginfo_t *info, void *ucontext)
> >  {
> >  	int operation, ret, malloc_and_init_memory, memflush;
> > -	unsigned long span, buffer_span;
> > +	size_t span, buffer_span;
> >  	char **benchmark_cmd;
> >  	char resctrl_val[64];
> >  	FILE *fp;
> 
> Do we now need a cast in the initialization of span?

I don't see any warning w/o cast, unsigned long -> size_t seems pretty 
safe anyway. For internally provided values, overflow does not seem 
possible even if the type sizes would disagree.

There's no existing error checking for the command in the case where 
"fill_buf" is used with -b (an orthogonal issue).

-- 
 i.