[webkit-reviews] review denied: [Bug 81861] [chromium/FileWriter] two race conditions : [Attachment 133164] Patch

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Eric U. <ericu at chromium.org>'s
request for review:
Bug 81861: [chromium/FileWriter] two race conditions
https://bugs.webkit.org/show_bug.cgi?id=81861

Attachment 133164: Patch
https://bugs.webkit.org/attachment.cgi?id=133164&action=review

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=133164&action=review


I agree with Kinuko so this at least needs a few more comments (in the
ChangeLog).

> Source/WebCore/Modules/filesystem/FileWriter.cpp:127
> +    } else {

doOperation(OperationWrite);

> Source/WebCore/Modules/filesystem/FileWriter.cpp:168
> +    } else {

doOperation(OperationTruncate);

>> Source/WebCore/Modules/filesystem/FileWriter.cpp:195
>> +	    completeAbort();
> 
> Can we early exit here and remove else in line 196?

And that should make the diff less as well :)

> Source/WebCore/Modules/filesystem/FileWriter.cpp:281
> +    default:

Don't add a default to a switch for an enum so that the compile will catch
unhandled enums. 

In this case if you wish to assert, do it after the switch and change the
break's to return's so they don't hit the assert (and add the new enum value in
here and just break for it).