[PATCH] D78897: [Support] raw_fd_ostream can lock file before write

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 27 05:52:20 PDT 2020


labath added a reviewer: dblaikie.
labath added a comment.

While I don't really feel qualified to set the direction here, I gotta say that this patch (and the built-in timeout in particular) looks worrying to me.

Overall I feel that this sort of automatic locking will very rarely be the "right tool for the job". I think your follow-up patch sort of demonstrates that, because you've needed to format the output to a temporary string stream in order for it do what you need. I think it might be more reasonable to just leave this out of the raw_ostream class, and do a manual `lockFile` + `write` + `unlockFile` combo where needed. In fact, if we do that, I'm wondering if locking is really needed, as a write to a `O_APPEND` file is more-or-less guaranteed to be atomic (I think the only problem comes from signals, and I'm not sure if those really happen on local files):

  O_APPEND
         The  file is opened in append mode.  Before each write(2), the file offset is positioned
         at the end of the file, as if with lseek(2).  The modification of the  file  offset  and
         the write operation are performed as a single atomic step.

I believe a similar effect on windows can be achieved with `FILE_APPEND_DATA` and/or writing to offset 0xffffffffffffffff

  FILE_APPEND_DATA
  	For a file object, the right to append data to the file. (For local files, write operations will not overwrite existing data if this flag is specified without FILE_WRITE_DATA.)
  
  WriteFile:
          To write to the end of file, specify both the Offset and OffsetHigh members of the OVERLAPPED structure as 0xFFFFFFFF. This is functionally equivalent to previously calling the CreateFile function to open hFile using FILE_APPEND_DATA access.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78897/new/

https://reviews.llvm.org/D78897





More information about the llvm-commits mailing list