Page MenuHomePhabricator

[Support] Add docs for 'openFileFor{Write,Read}'
ClosedPublic

Authored by modocache on May 5 2018, 9:11 PM.

Details

Summary

Add documentation for the LLVM Support functions openFileForWrite and
openFileForRead. The openFileForRead parameter RealPath, in
particular, I think warranted some explanation.

In addition, make the behavior of the functions more consistent across
platforms. Prior to this patch, Windows would set or not set the result
file descriptor based on the nature of the error, whereas Unix would
consistently set it to -1 if the open failed. Make Windows
consistently set it to -1 as well.

Test Plan:

  1. ninja check-llvm
  2. ninja docs-llvm-html

Diff Detail

Repository
rL LLVM

Event Timeline

modocache created this revision.May 5 2018, 9:11 PM
danielmartin accepted this revision.May 9 2018, 8:48 AM
danielmartin added a subscriber: danielmartin.

LGTM.

This revision is now accepted and ready to land.May 9 2018, 8:48 AM
scanon added a subscriber: scanon.May 9 2018, 8:52 AM

Two quick notes as someone who has never used these functions:

  • Name is really the *path* of the file to open, right?
  • What happens to ResultFD if the function fails?
modocache added a comment.EditedMay 9 2018, 8:55 AM

Thanks for the comments!

Name is really the *path* of the file to open, right?

Correct, it's the path. It might be relative or absolute. I'll update the docs (but not the variable name).

What happens to ResultFD if the function fails?

Good point. On Unix it's set to the return value of open(2). I'll double check what happens on Windows and update the docs.

scanon added a comment.May 9 2018, 8:57 AM

One more question: the caller is responsible for closing the file when they're done, right?

Correct! I'll mention that as well.

modocache updated this revision to Diff 146224.May 10 2018, 2:17 PM
modocache edited the summary of this revision. (Show Details)

I updated this diff to not only document how the ResultFD is set, but also make it a little more consistent across platforms. Previously Windows would only sometimes set it to -1 in the case of failure, now it always does.

scanon accepted this revision.May 10 2018, 2:19 PM

LGTM, thanks!

This revision was automatically updated to reflect the committed changes.