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

RESOLVED FIXED in mozilla17

Status

()

Core
Networking: File
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Yoric, Assigned: Yoric)

Tracking

unspecified
mozilla17
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 5 obsolete attachments)

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.
Depends on: 750178
Severity: normal → enhancement
OS: Mac OS X → All
Hardware: x86 → All
Created attachment 641868 [details] [diff] [review]
1. Defining libc, OS.File.Error in their own file (Unix).
Assignee: nobody → dteller
Attachment #641868 - Flags: review?(taras.mozilla)
Created attachment 641870 [details] [diff] [review]
2. Adapting osfile_* (Unix)
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).
Created attachment 641871 [details] [diff] [review]
3. Defining libc, OS.File.Error in their own file (Win)
Created attachment 641872 [details] [diff] [review]
4. Adapting osfile_* (Win)
Created attachment 641874 [details] [diff] [review]
Companion makefile
Blocks: 773643

Updated

5 years ago
Attachment #641868 - Flags: review?(taras.mozilla) → review+

Updated

5 years ago
Attachment #641870 - Flags: review?(taras.mozilla) → review+
Attachment #641871 - Flags: review?(taras.mozilla)

Comment 7

5 years ago
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)

Updated

5 years ago
Attachment #641872 - Flags: review?(taras.mozilla) → review+
Blocks: 775950
Depends on: 766194
Blocks: 776258
Created attachment 646124 [details] [diff] [review]
Shared code

I will take advantage of the opportunity to rename files.
Attachment #641874 - Attachment is obsolete: true
Attachment #646124 - Flags: review+
Created attachment 646125 [details] [diff] [review]
Unix errors
Attachment #641868 - Attachment is obsolete: true
Attachment #646125 - Flags: review+
Created attachment 646126 [details] [diff] [review]
2. Adapting osfile_* (Unix)
Attachment #641870 - Attachment is obsolete: true
Attachment #646126 - Flags: review+
Created attachment 646127 [details] [diff] [review]
Windows errors
Attachment #641871 - Attachment is obsolete: true
Attachment #646127 - Flags: review+
Created attachment 646128 [details] [diff] [review]
4. Adapting osfile_* (Win)
Attachment #641872 - Attachment is obsolete: true
Attachment #646128 - Flags: review+
Keywords: checkin-needed
Blocks: 772187
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ca5142e757d
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b931642e993
https://hg.mozilla.org/integration/mozilla-inbound/rev/48fb9e5ec22d
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ff952f8a4c7
https://hg.mozilla.org/integration/mozilla-inbound/rev/e255eaa167fa

Should this have tests?
Flags: in-testsuite?
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3ca5142e757d
https://hg.mozilla.org/mozilla-central/rev/3b931642e993
https://hg.mozilla.org/mozilla-central/rev/48fb9e5ec22d
https://hg.mozilla.org/mozilla-central/rev/8ff952f8a4c7
https://hg.mozilla.org/mozilla-central/rev/e255eaa167fa
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
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.