[PATCH] D39710: Simplify unlinkAsync

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 7 11:08:42 PST 2017


Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:

> ruiu added inline comments.
>
>
> ================
> Comment at: ELF/Filesystem.cpp:19
>  #include "llvm/Support/FileSystem.h"
> +#include <unistd.h>
>  
> ----------------
> Does all system have unistd.h?

Looks like I have to check defined(HAVE_UNISTD_H). I will add that.

>
>
> ================
> Comment at: ELF/Filesystem.cpp:54
>  
> -  // Remove TempPath in background.
> -  runBackground([=] { ::remove(TempPath.str().str().c_str()); });
> +  sys::fs::remove(Path);
> +
> ----------------
> Is this safe on Windows? On Windows, you can't remove opened files.

It depends on how they are opened. We use FILE_SHARE_DELETE in
openFileForRead, so it should work.

But in fact, remove being slow is a unix only problem, so I guess the
best thing to do is to return early on windows. I will add that too.

Thanks,
Rafael


More information about the llvm-commits mailing list