786196 - [OS.File] Implement flushing

Status: RESOLVED FIXED

(Toolkit Graveyard :: OS.File, defect)

Description

[David Teller [:Yoric] - still alive but not very active]

We need an operation |OS.File.prototype.flush|.
This operation should be implemented with:
- |fsync| under Unix;
- |FlushFileBuffers| under Windows.

Getting in the code of OS.File might be tricky, but implementation should be easy, so marking this as a mentored bug.

Comment 1

[Luqman Aden [:Luqman]]

Attachment: [OS.File] Implement flushing

Comment 2

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 656116 [details] [diff] [review]
[OS.File] Implement flushing

Review of attachment 656116 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good, thanks for the patch.

Comment 3

[David Teller [:Yoric] - still alive but not very active]

It might be useful to add a trivial testsuite that ensures that |foo.flush()| does not cause an error, where |foo| is an open file.

Comment 4

[Luqman Aden [:Luqman]]

Attachment: [OS.File] Implement flushing

Added simple testcase.

Comment 5

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 656166 [details] [diff] [review]
[OS.File] Implement flushing

Review of attachment 656166 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. However, I am not sure whether flushing works on files opened for reading. Just in case it doesn't, could you adapt the test suite to files opened for writing?

Comment 6

[Luqman Aden [:Luqman]]

Attachment: [OS.File] Implement flushing

Comment 7

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 656171 [details] [diff] [review]
[OS.File] Implement flushing

Review of attachment 656171 [details] [diff] [review]:
-----------------------------------------------------------------

Er... please don't overwrite important files during the test :)

Comment 8

[Luqman Aden [:Luqman]]

Attachment: [OS.File] Implement flushing

No more overwriting important files

Comment 9

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 656188 [details] [diff] [review]
[OS.File] Implement flushing

Review of attachment 656188 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Could you just call the file test_flush.tmp (no directory component) to match other files created by the test? No need to re-ask me for a review for this.

Comment 10

[Luqman Aden [:Luqman]]

Attachment: [OS.File] Implement flushing

Comment 11

[Luqman Aden [:Luqman]]

Attachment: [OS.File] Implement flushing

Comment 12

[Luqman Aden [:Luqman]]

https://tbpl.mozilla.org/?tree=Try&rev=480840907ab5

Comment 13

[Ryan VanderMeulen [:RyanVM]]

(In reply to Luqman Aden from comment #12)
> https://tbpl.mozilla.org/?tree=Try&rev=480840907ab5

Green on Try.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e53491a235ca

Comment 14

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/mozilla-central/rev/e53491a235ca