linux-kernel - Re: [PATCH v4 09/19] selftests/resctrl: Convert span to size

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 14 Jul 2023 13:33:25 +0300 (EEST)
From:   Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To:     Reinette Chatre <reinette.chatre@...el.com>
cc:     linux-kselftest@...r.kernel.org, Shuah Khan <shuah@...nel.org>,
        Shaopeng Tan <tan.shaopeng@...fujitsu.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 09/19] selftests/resctrl: Convert span to size_t

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.