Closed Bug 769312 Opened 10 years ago Closed 10 years ago

[OS.File] Separate OS.File.Error from worker-only code

Categories

(Core :: Networking: File, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: Yoric, Assigned: Yoric)

References

Details

Attachments

(5 files, 5 obsolete files)

2.33 KB, patch
Yoric
: review+
Details | Diff | Splinter Review
4.57 KB, patch
Yoric
: review+
Details | Diff | Splinter Review
9.38 KB, patch
Yoric
: review+
Details | Diff | Splinter Review
5.11 KB, patch
Yoric
: review+
Details | Diff | Splinter Review
8.32 KB, patch
Yoric
: review+
Details | Diff | Splinter Review
At the moment, OS.File.Error is tightly coupled with worker-only code. This is not a good idea, as we will want to represent I/O errors also on the main thread.
Note a subtlety:
- the Unix version requires OS.Unix.strerror;
- the Windows version requires OS.Win.FormatMessage, OS.Constants.Win.FORMAT_MESSAGE_FROM_SYSTEM, OS.Constants.Win.FORMAT_MESSAGE_IGNORE_INSERTS.
Severity: normal → enhancement
OS: Mac OS X → All
Hardware: x86 → All
Assignee: nobody → dteller
Attachment #641868 - Flags: review?(taras.mozilla)
Attached patch 2. Adapting osfile_* (Unix) (obsolete) — Splinter Review
Attachment #641870 - Flags: review?(taras.mozilla)
Attachment #641868 - Attachment description: 1. Defining libc, OS.File.Error in their own file. → 1. Defining libc, OS.File.Error in their own file (Unix).
Attachment #641868 - Flags: review?(taras.mozilla) → review+
Attachment #641870 - Flags: review?(taras.mozilla) → review+
Attachment #641871 - Flags: review?(taras.mozilla)
Comment on attachment 641871 [details] [diff] [review]
3. Defining libc, OS.File.Error in their own file (Win)

no need to break out separate patches for each file.
Attachment #641871 - Flags: review?(taras.mozilla) → review+
Attachment #641872 - Attachment is patch: true
Attachment #641872 - Flags: review?(taras.mozilla)
Attachment #641872 - Flags: review?(taras.mozilla) → review+
Attached patch Shared codeSplinter Review
I will take advantage of the opportunity to rename files.
Attachment #641874 - Attachment is obsolete: true
Attachment #646124 - Flags: review+
Attached patch Unix errorsSplinter Review
Attachment #641868 - Attachment is obsolete: true
Attachment #646125 - Flags: review+
Attachment #641870 - Attachment is obsolete: true
Attachment #646126 - Flags: review+
Attached patch Windows errorsSplinter Review
Attachment #641871 - Attachment is obsolete: true
Attachment #646127 - Flags: review+
Attachment #641872 - Attachment is obsolete: true
Attachment #646128 - Flags: review+
Small request: please add commit messages with a bit more detail as to function to patches. This landed as:

  David Rajchenbach-Teller — Bug 769312 - Adapt the rest of the code (Windows side).
  David Rajchenbach-Teller — Bug 769312 - Windows errors. r=taras
  David Rajchenbach-Teller — Bug 769312 - Adapt the rest of the code (Unix side).
  David Rajchenbach-Teller — Bug 769312 - Unix errors. r=taras
  David Rajchenbach-Teller — Bug 769312 - Shared code. r=yoric

...which isn't very useful for tree-watching or future file changelog digging. Bug titles are generally enough (and easy!).
You need to log in before you can comment on or make changes to this bug.