On Thu, 2014-12-11 at 18:12 -0300, Arnaldo Carvalho de Melo wrote:
> > +struct timeval start, end, total;
> > +static unsigned int start_nr = 100;
> > +static unsigned int end_nr = 1100;
> > +static unsigned int increment_by = 100;
> > +static int bench_dur = NOTSET;
> > +static int num_jobs = NOTSET;
> > +static bool run_jobs;
> > +static int n_pro;
> > +
> > +/* Shared variables between fork processes*/
> > +unsigned int *finished, *setup;
> > +unsigned long long *shared_workers;
> > +char *tmp_dir;
>
> Are you sure these variables aren't static?
>
Yes you are right. I will change these to static.
> > +/* Running bench vfs workload */
> > +static void *run_bench_vfs(struct worker *workers)
> > +{
> > + int fd;
> > + unsigned long long nr_ops = 0;
> > + int jobs = num_jobs;
> > +
> > + sprintf(workers->str, "%s/%d-XXXXXX", tmp_dir, getpid());
>
>
> Please use snprintf, checking for overflows on the target string
>
Thank you for pointing this out. I will change all sprintf to use
snprintf and check for overflows on the target string.
> > +
> > + /* finished shared var is use to signal start and end of benchmark */
> > + finished_tmp = (void *)mmap(0, sizeof(unsigned int),
> > PROT_READ|PROT_WRITE,
> > + MAP_SHARED|MAP_ANONYMOUS, -1, 0);
>
> Why do you use these void * casts before mmap alreayd returns void *?
>
Yes the cast to void here is redundant. I will remove the cast to (void
*) here.
> > + if (finished_tmp == (void *) -1)
>
> Please use MAP_FAILED instead of its equivalent (void *) -1.
>
I will check for MAP_FAILED here instead. Thank you for the suggestion.
-Tuan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
