> On Dec. 4, 2013, 10:18 p.m., Andrew Stitcher wrote: > > I can see no actual justification for this change in either the review > > comments or the refered bug: > > > > There is nothing per se wrong with this change, I just don't really > > understand why it necessary. > > > > Incidentally '/' works perfectly well as a path separator on windows too > > (even if it is not the default). It is true that the test for an absolute > > path would be different though. > >
It's required because AclPlugin falls over if you give it an aclFile of the form c:\path\to\aclfile. It currently tests for absolute path with path[0]=='/'. The rest of Path is not immediately essential but I though we could use a place to collect other path-related portability fixes in the future. I looked at FileSysDir but it's quite specifically for directories. We could refactor FileSysDir as a subclass of Path if you think that's worthwhile. This came up in the context of work on portable python-based tests, but I wanted to separate this out because it stands alone. I can create another bug for it if need be. > On Dec. 4, 2013, 10:18 p.m., Andrew Stitcher wrote: > > /trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp, line 75 > > <https://reviews.apache.org/r/16018/diff/1/?file=394139#file394139line75> > > > > "aclFile+dataDir" looks backwards. Even this is actually correct it is > > going to cause confusion. aclFile+dataDir is backwards! good catch, will fix that. - Alan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16018/#review29765 ----------------------------------------------------------- On Dec. 4, 2013, 10:06 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16018/ > ----------------------------------------------------------- > > (Updated Dec. 4, 2013, 10:06 p.m.) > > > Review request for qpid, Andrew Stitcher and Steve Huston. > > > Bugs: QPID-5307 > https://issues.apache.org/jira/browse/QPID-5307 > > > Repository: qpid > > > Description > ------- > > QPID-5307: Added portable Path, check for isAbsolute() > > qpid::sys::Path provides some basic operations on path names, > with implementations for posix and windows. > > Needed by work on a portable test system. > > > Diffs > ----- > > /trunk/qpid/cpp/src/CMakeLists.txt 1546628 > /trunk/qpid/cpp/src/qpid/acl/AclPlugin.cpp 1546628 > /trunk/qpid/cpp/src/qpid/sys/Path.h PRE-CREATION > /trunk/qpid/cpp/src/qpid/sys/posix/Path.cpp PRE-CREATION > /trunk/qpid/cpp/src/qpid/sys/windows/Path.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/16018/diff/ > > > Testing > ------- > > > Thanks, > > Alan Conway > >
