[libcxx-commits] [PATCH] D91140: [8/N] [libcxx] Fix the preexisting directory_iterator code for windows

Martin Storsjö via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Dec 14 23:37:50 PST 2020

mstorsjo added inline comments.

Comment at: libcxx/src/filesystem/directory_iterator.cpp:75
ldionne wrote:
> I think adding a comment here to signify that this is the Windows branch would add readability.
Sure, I can add that.

Comment at: libcxx/src/filesystem/directory_iterator.cpp:121
+    }
+    __stream_ = ::FindFirstFileW((root / "*").c_str(), &__data_);
     if (__stream_ == INVALID_HANDLE_VALUE) {
ldionne wrote:
> Isn't that a use-after-free? `(root / "*").c_str()` returns a pointer to the internal string held in the temporary object created by `(root / "*")`, unless I'm missing a subtlety here. I don't think the lifetime of objects in an expression used as a function argument is extended automatically.
I don't know the spec bits exactly that allow this, but I'm fairly sure it's supposed to work. An empirical test: https://godbolt.org/z/za8K46

I'm not too familiar with the C++ spec itself to reference it myself, but https://stackoverflow.com/a/584835/3115956 claims that "12.2 Temporary objects" in the C++ standard covers this, saying that the destructors for such temporaries aren't called until at the end of the full-expression.

Comment at: libcxx/src/filesystem/filesystem_common.h:21
+#if !defined(_LIBCPP_WIN32API)
 #include <unistd.h>
 #include <sys/stat.h>
ldionne wrote:
> Please indent `#include`s in between.
Ok, will do that for this block I'm touching here. There's more unindented blocks in the same file, but I'm keeping them untouched here.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list