OS.File.writeAtomic doesn't handle file open error for some writes

RESOLVED FIXED in mozilla25

Status

()

Toolkit
OS.File
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Irving, Assigned: Irving)

Tracking

unspecified
mozilla25
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 770867 [details] [diff] [review]
Null check the destination file before closing

After the recent sync-write-removal patch, OS.File.writeAtomic no longer reports errors correctly for some file opens.

https://hg.mozilla.org/mozilla-central/file/2cae857c17cb/toolkit/components/osfile/osfile_shared_front.jsm#l362

   359   if (!options.flush) {
   360     // Just write, without any renaming trick
   361     let dest;
   362     try {
   363       dest = OS.File.open(path, {write: true, truncate: true});
   364       return dest.write(buffer, options);
   365     } finally {
   366       dest.close();
   367     }
   368   }

If OS.File.open() fails, the finally() block throws a JS error because dest is undefined.
Attachment #770867 - Flags: review?(dteller)
Comment on attachment 770867 [details] [diff] [review]
Null check the destination file before closing

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

::: toolkit/components/osfile/osfile_shared_front.jsm
@@ +368,2 @@
>      }
>    }

Actually, we could just move the |open| out of the try.
Attachment #770867 - Flags: review?(dteller) → feedback+
Created attachment 770902 [details] [diff] [review]
Move Open() outside the try/finally

Or we could do it this way...
Attachment #770867 - Attachment is obsolete: true
Attachment #770902 - Flags: review?(dteller)
https://hg.mozilla.org/mozilla-central/rev/a828fef4d274
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.