[lld] r222799 - [PECOFF] Properly close a file descriptor.

[David Blaikie]

On Tue, Nov 25, 2014 at 4:32 PM, Rui Ueyama <ruiu at google.com> wrote:

> Yeah, I was looking for a function to close the file descriptor, but seem
> there's no such function, so I did this way. Does this really not exist? If
> so, I agree that we should add the function to llvm::sys::fs.
>

Seems raw_ostream uses "lib/Support/raw_ostream.cpp: if
(sys::Process::SafelyCloseFileDescriptor(FD))" - so
sys::Process::SafelyCloseFileDescriptor is about as close to a good way to
do this as exists, maybe?

I don't know these libraries very well - just my cursory poking around, at
least.


>
> On Tue, Nov 25, 2014 at 4:30 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Tue, Nov 25, 2014 at 4:21 PM, Rui Ueyama <ruiu at google.com> wrote:
>>
>>> Author: ruiu
>>> Date: Tue Nov 25 18:21:27 2014
>>> New Revision: 222799
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=222799&view=rev
>>> Log:
>>> [PECOFF] Properly close a file descriptor.
>>>
>>> This was basically benign resource leak on Unix, but on Windows
>>> it could cause builds to fail because opened file descriptor
>>> prevents other processes from moving or removing the file.
>>>
>>> Modified:
>>>     lld/trunk/lib/ReaderWriter/PECOFF/PDBPass.h
>>>
>>> Modified: lld/trunk/lib/ReaderWriter/PECOFF/PDBPass.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/PDBPass.h?rev=222799&r1=222798&r2=222799&view=diff
>>>
>>> ==============================================================================
>>> --- lld/trunk/lib/ReaderWriter/PECOFF/PDBPass.h (original)
>>> +++ lld/trunk/lib/ReaderWriter/PECOFF/PDBPass.h Tue Nov 25 18:21:27 2014
>>> @@ -13,6 +13,12 @@
>>>  #include "lld/Core/Pass.h"
>>>  #include "llvm/ADT/StringRef.h"
>>>
>>> +#if !defined(_MSC_VER) && !defined(__MINGW32__)
>>> +#include <unistd.h>
>>> +#else
>>> +#include <io.h>
>>> +#endif
>>> +
>>>  namespace lld {
>>>  namespace pecoff {
>>>
>>> @@ -30,6 +36,7 @@ private:
>>>      int fd;
>>>      if (llvm::sys::fs::openFileForWrite(path, fd,
>>> llvm::sys::fs::F_Append))
>>>        llvm::report_fatal_error("failed to create a PDB file");
>>> +    ::close(fd);
>>>
>>
>> This seems asymmetric - using an llvm::sys::fs API to open it, but using
>> the raw OS API to close? Should we roll this into llvm::sys::fs alongside
>> the opening functions?
>>
>>
>>>    }
>>>
>>>    PECOFFLinkingContext &_ctx;
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141125/79221480/attachment.html>