----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34256/#review84375 -----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp <https://reviews.apache.org/r/34256/#comment135590> Can you add comments to the code here? It is really hard to follow what's happening. Alternatively, can you simplify these using "strings" functions? For example, looking at http://linux.die.net/man/3/basename ``` The functions dirname() and basename() break a null-terminated pathname string into directory and filename components. In the usual case, dirname() returns the string up to, but not including, the final '/', and basename() returns the component following the final '/'. Trailing '/' characters are not counted as part of the pathname. If path does not contain a slash, dirname() returns the string "." while basename() returns a copy of path. If path is the string "/", then both dirname() and basename() return the string "/". If path is a NULL pointer or points to an empty string, then both dirname() and basename() return the string ".". ``` this is how I would implement basename ``` string basename() { // Remove trailing "/", if exists. string result = strings::remove(value, "/", strings::SUFFIX); if (result.empty()) { return string("."); } if (!strings::contains(result, "/")) { return result; } if (result == "/") { return result; } vector<string> tokens = strings::tokenize(result, "/"); return tokens[tokens.size() - 1]; } ``` I haven't checked all the edge cases, but you get the idea. 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp <https://reviews.apache.org/r/34256/#comment135593> nice tests. - Vinod Kone On May 17, 2015, 7:46 p.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34256/ > ----------------------------------------------------------- > > (Updated May 17, 2015, 7:46 p.m.) > > > Review request for mesos and Cody Maloney. > > > Bugs: MESOS-1303 > https://issues.apache.org/jira/browse/MESOS-1303 > > > Repository: mesos-incubating > > > Description > ------- > > Introducing Path::dirname() and Path::basename() as a thread safe replacement > of the respective system functions. Also contains new tests covering corner > cases. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp d4df650 > 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp cf35412 > > Diff: https://reviews.apache.org/r/34256/diff/ > > > Testing > ------- > > make check (including new tests). > > Result comparison to match ::dirname and ::basename on interesting cases. > > > Thanks, > > Till Toenshoff > >
