Last Comment Bug 779347 - wrap OpenFile and ReadCompleteFile in a nice RAII interface
: wrap OpenFile and ReadCompleteFile in a nice RAII interface
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: :Benjamin Peterson
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-31 16:18 PDT by :Benjamin Peterson
Modified: 2012-08-01 02:52 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
add FileWrapper (3.64 KB, patch)
2012-07-31 16:19 PDT, :Benjamin Peterson
jimb: review+
Details | Diff | Splinter Review

Description :Benjamin Peterson 2012-07-31 16:18:49 PDT
(Bug 779038 followup)

This patch provides a class that automatically handles the closing (or not closing of stdin) of FILE* jsapi creates.
Comment 1 :Benjamin Peterson 2012-07-31 16:19:59 PDT
Created attachment 647749 [details] [diff] [review]
add FileWrapper
Comment 2 Jim Blandy :jimb 2012-07-31 17:18:32 PDT
Comment on attachment 647749 [details] [diff] [review]
add FileWrapper

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

Hurrah! Some comments; see what you think.

::: js/src/jsapi.cpp
@@ +5057,5 @@
>  
>      return true;
>  }
>  
> +class FileWrapper

It's my impression that classes intended mostly to guarantee cleanup from some local scope get names beginning with "Auto".

@@ +5085,5 @@
>   * Open a source file for reading. Supports "-" and NULL to mean stdin. The
>   * return value must be fclosed unless it is stdin.
>   */
> +static bool
> +OpenFile(JSContext *cx, const char *filename, FileWrapper &fw)

I think OpenFile would work nicely as a member function of FileWrapper.

@@ +5669,5 @@
> +    {
> +        FileWrapper fw;
> +        if (!OpenFile(cx, filename, fw))
> +            return NULL;
> +        bool ok = fw.readAll(cx, buffer);

There's no need for 'ok' any more, since the fclose is gone.
Comment 3 :Benjamin Peterson 2012-07-31 17:37:05 PDT
I
- zapped ok
- renamed FileWrapper -> AutoFile
- OpenFile -> AutoFile::open

https://hg.mozilla.org/integration/mozilla-inbound/rev/162b401d8774
Comment 4 Jim Blandy :jimb 2012-07-31 17:39:03 PDT
Yeah, that's the stuff.
Comment 5 Ed Morley [:emorley] 2012-08-01 02:52:26 PDT
https://hg.mozilla.org/mozilla-central/rev/162b401d8774

Note You need to log in before you can comment on or make changes to this bug.