A comment on why we want this bit: "OutPath.isRegularFile() && OutPath.canWrite()" would be good, but otherwise looks great!
Thanks, - Daniel On Thu, Sep 16, 2010 at 2:23 PM, Argyrios Kyrtzidis <[email protected]> wrote: > New patch attached. > > -Argiris > > > > On Sep 16, 2010, at 4:13 PM, Daniel Dunbar wrote: > >> Hi Argiris, >> >> Looks pretty good, two comments on createOutputFile though. >> >> We should still go ahead and unlink the output file immediately, >> even when using temporary files. We shouldn't leave the output >> file around in the case of failure. This should make the test >> suite change in this patch unnecessary. >> >> Also, I personally think >>> + std::string OSFile = OutFile; >> ... >>> + OSFile = TempFile; >> is easier to follow than using the ternary operator, but that's me. >> >> - Daniel >> >> On Thu, Sep 16, 2010 at 6:30 AM, Argyrios Kyrtzidis <[email protected]> >> wrote: >>> Attached patch uses a temporary file for output which gets renamed after all >>> the writing is finished. This mainly prevents failures and/or crashes when >>> multiple processes try to read/write the same PCH file. >>> (rdar://8392711&8294781) >>> Please review. >>> -Argiris >>> >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >>> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
