> On Oct. 20, 2014, 1:56 p.m., Niklas Nielsen wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 1283 > > <https://reviews.apache.org/r/26903/diff/2/?file=725229#file725229line1283> > > > > How about flatten it to os::libraries::expand() ?
It's not clear what expand() would mean but expandName() is clearer. expandLibraryName() would have been the best, though. > On Oct. 20, 2014, 1:56 p.m., Niklas Nielsen wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 1271 > > <https://reviews.apache.org/r/26903/diff/2/?file=725229#file725229line1271> > > > > I looked through your patch set - it doesn't look like you use this > > function anywhere? > > > > It is a bit unclear to use 'ld library path' and 'path' > > interchangeably. How about sticking to 'path' or 'paths' (it is a path list > > right?) > > > > For example: > > > > // returns ':' separated list of library paths. > > os::libraries::paths() > > > > // Resets library variable to ':' separated library list. > > os::libraries::setPaths() > > > > // Appends library path to library list. > > os::libraries::addPath() > > > > Let's think about the path() overload for a little bit. I think for > > getting and setting, we should call out explicitly what we do ( > > > > http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Overloading). We use ldLibraryPath() to get the corresponding environment variable string ("LD_LIBRARY_PATH"/"DYLD_LIBRARY_PATH"). I like the paths(), setPaths() and addPaths() function names and will update the diff accordingly. - Kapil ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26903/#review57372 ----------------------------------------------------------- On Oct. 18, 2014, 3:04 p.m., Kapil Arya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26903/ > ----------------------------------------------------------- > > (Updated Oct. 18, 2014, 3:04 p.m.) > > > Review request for mesos, Benjamin Hindman and Niklas Nielsen. > > > Repository: mesos-git > > > Description > ------- > > Also added a utility function to expand a library name to a filename > by prefixing "lib" and adding the default extension. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp > 63bda7a87e24e1696f3e7bb0206c551ce39dfe27 > > Diff: https://reviews.apache.org/r/26903/diff/ > > > Testing > ------- > > Ran make check. > > > Thanks, > > Kapil Arya > >
