> On Nov. 2, 2016, 4:39 p.m., Benjamin Bannier wrote: > > src/jvm/jvm.cpp, lines 97-102 > > <https://reviews.apache.org/r/53386/diff/1/?file=1551496#file1551496line97> > > > > This will work as long nobody adds new `return` statements before the > > corresponding `delete`. > > > > Could we use a `std::vector<JavaVMOption>` here for easier maintenance? > > Note that you can access the raw memory block of the entries like > > > > // conditional on `options.size() != 0` > > vmArgs.options = &opts[0];
It is a bit annoying to maintain, although that is true for any situation in which we're using `new`/`delete`, which unfortunately is quite common throughout the code base. Personally I think `unique_ptr<JavaVMOption>` would be the best choice (I believe we can't use `Owned` since it doesn't have the right specialization to use `delete[]` when necessary). Using `vector` here seems a bit unfortunate, since the array is a fixed size; I also feel a bit squimish about accessing the underlying raw memory block. - Neil ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53386/#review154578 ----------------------------------------------------------- On Nov. 2, 2016, 4:23 p.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53386/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2016, 4:23 p.m.) > > > Review request for mesos and Joseph Wu. > > > Repository: mesos > > > Description > ------- > > Fixed memory leak in JVM code. > > > Diffs > ----- > > src/jvm/jvm.cpp 62f7857180162c510269b5d10f5add94ab354fa2 > > Diff: https://reviews.apache.org/r/53386/diff/ > > > Testing > ------- > > `make check` > > Verified that observed `clang-tidy` leak warning goes away with this change. > > > Thanks, > > Neil Conway > >
