On Fri, Jul 7, 2023 at 10:03 AM Axel Rasmussen <axelrasmussen@xxxxxxxxxx> wrote:
>
> On Fri, Jul 7, 2023 at 6:42 AM Peter Xu <peterx@xxxxxxxxxx> wrote:
> >
> > On Thu, Jul 06, 2023 at 03:50:35PM -0700, Axel Rasmussen wrote:
> > > @@ -247,9 +245,13 @@ static int userfaultfd_stress(void)
> > > {
> > > void *area;
> > > unsigned long nr;
> > > - struct uffd_args args[nr_cpus];
> > > + struct uffd_args *args;
> > > uint64_t mem_size = nr_pages * page_size;
> > >
> > > + args = calloc(nr_cpus, sizeof(struct uffd_args));
> > > + if (!args)
> > > + err("allocating args array failed");
> >
> > This is trivial, but I think I requested a "free" (or keep it allocate on
> > stack) in previous version but it didn't get a response on why we cannot
> > and it kept going.. could you help explain?
>
> Oh, sorry! I had meant to change this after our discussion, and simply
> overlooked it while reworking the patches.
>
> I'll include this change in a v4 which also addresses e.g. the
> comments on commit 1.
Ah, so I tried switching back to the {0} initializer, and was reminded
why I didn't do that in v1. :) Ignoring the missing braces warning I
talked about before, using {0} here is actually an error
("variable-sized object may not be initialized") because this is a
variable sized array (nr_cpus isn't constant). So, that option is out.
I'm not a huge fan of adding the free() cleanup and dealing with all
of the err() calls this function has.
Originally I switched to calloc() because I'm not a big fan of VLAs
anyway. But, as a compromise in v4 I'll leave it a VLA, and switch to
memset() for initializing it.
>
> >
> > --
> > Peter Xu
> >
