Minor nit: your diff is much bigger than it should have been due to whitespace changes.
On Sun, Feb 5, 2017 at 9:21 AM, Saleem Abdulrasool via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: compnerd > Date: Sun Feb 5 11:21:52 2017 > New Revision: 294127 > > URL: http://llvm.org/viewvc/llvm-project?rev=294127&view=rev > Log: > filesystem: fix n4100 conformance for `temp_directory_path` > > N4100 states that an error shall be reported if > `!exists(p) || !is_directory(p)`. We were missing the first half of the > conditional. Invert the error and normal code paths to make the code > easier to follow. > > Modified: > libcxx/trunk/src/experimental/filesystem/operations.cpp > libcxx/trunk/test/std/experimental/filesystem/fs.op. > funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp > > Modified: libcxx/trunk/src/experimental/filesystem/operations.cpp > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/ > experimental/filesystem/operations.cpp?rev=294127&r1= > 294126&r2=294127&view=diff > ============================================================ > ================== > --- libcxx/trunk/src/experimental/filesystem/operations.cpp (original) > +++ libcxx/trunk/src/experimental/filesystem/operations.cpp Sun Feb 5 > 11:21:52 2017 > @@ -886,23 +886,28 @@ path __system_complete(const path& p, st > return absolute(p, current_path()); > } > > -path __temp_directory_path(std::error_code *ec) { > - const char* env_paths[] = {"TMPDIR", "TMP", "TEMP", "TEMPDIR"}; > - const char* ret = nullptr; > - for (auto & ep : env_paths) { > - if ((ret = std::getenv(ep))) > - break; > - } > - path p(ret ? ret : "/tmp"); > - std::error_code m_ec; > - if (is_directory(p, m_ec)) { > - if (ec) ec->clear(); > - return p; > - } > +path __temp_directory_path(std::error_code* ec) { > + const char* env_paths[] = {"TMPDIR", "TMP", "TEMP", "TEMPDIR"}; > + const char* ret = nullptr; > + > + for (auto& ep : env_paths) > + if ((ret = std::getenv(ep))) > + break; > + if (ret == nullptr) > + ret = "/tmp"; > + > + path p(ret); > + std::error_code m_ec; > + if (!exists(p, m_ec) || !is_directory(p, m_ec)) { > if (!m_ec || m_ec == make_error_code(errc::no_such_ > file_or_directory)) > - m_ec = make_error_code(errc::not_a_directory); > + m_ec = make_error_code(errc::not_a_directory); > set_or_throw(m_ec, ec, "temp_directory_path"); > return {}; > + } > + > + if (ec) > + ec->clear(); > + return p; > } > > // An absolute path is composed according to the table in > [fs.op.absolute]. > > Modified: libcxx/trunk/test/std/experimental/filesystem/fs.op. > funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp > URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/ > experimental/filesystem/fs.op.funcs/fs.op.temp_dir_path/ > temp_directory_path.pass.cpp?rev=294127&r1=294126&r2=294127&view=diff > ============================================================ > ================== > --- libcxx/trunk/test/std/experimental/filesystem/fs.op. > funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp (original) > +++ libcxx/trunk/test/std/experimental/filesystem/fs.op. > funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp Sun Feb 5 > 11:21:52 2017 > @@ -97,6 +97,14 @@ TEST_CASE(basic_tests) > TEST_CHECK(ec == std::make_error_code(std:: > errc::permission_denied)); > TEST_CHECK(ret == ""); > > + // Set the env variable to point to a non-existent dir > + PutEnv(TC.name, TC.p / "does_not_exist"); > + ec = GetTestEC(); > + ret = temp_directory_path(ec); > + TEST_CHECK(ec != GetTestEC()); > + TEST_CHECK(ec); > + TEST_CHECK(ret == ""); > + > // Finally erase this env variable > UnsetEnv(TC.name); > } > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits