[NaCl SDK] Add HTTP mount.

[NaCl SDK] Add HTTP mount.

BUG=156781
R=noelallen@chromium.org
NOTRY=true


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=177823

[noelallen1, 2013-01-14 23:53:41]

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
File native_client_sdk/src/libraries/nacl_mounts/mount_http.cc (right):

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:46: State state;
Uninitialized?

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:78: }
Can we assume no whitespace after \n?  Such as \t ' ' or a second \n?

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:149: std::vector<char>
buffer_;
When does buffer_ get sized?

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:203: char buffer[100];
100 bytes is a random size.

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:205:
snprintf(&buffer[0], 100, "bytes=%d-%d", offs, offs + count - 1);
%lu?

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:320: return 0;
Isn't this in a response header somewhere?

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:330: return
MountNode::Init(mode, uid, gid);
This always returns true...  Which means an open of a non-existent URL will
appear to be a failed Read.

[binji, 2013-01-15 01:42:48]

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
File native_client_sdk/src/libraries/nacl_mounts/mount_http.cc (right):

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:46: State state;
On 2013/01/14 23:53:41, noelallen1 wrote:
> Uninitialized?

Done.

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:78: }
On 2013/01/14 23:53:41, noelallen1 wrote:
> Can we assume no whitespace after \n?  Such as \t ' ' or a second \n?

Looks like trailing whitespace MAY be removed, but is not required. Probably
should handle it here though.

http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:149: std::vector<char>
buffer_;
On 2013/01/14 23:53:41, noelallen1 wrote:
> When does buffer_ get sized?

Done.

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:203: char buffer[100];
On 2013/01/14 23:53:41, noelallen1 wrote:
> 100 bytes is a random size.

It just needs to be big enough to fit the string below. Should I add a comment?

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:205:
snprintf(&buffer[0], 100, "bytes=%d-%d", offs, offs + count - 1);
On 2013/01/14 23:53:41, noelallen1 wrote:
> %lu?

Looks like the Google C++ style guide suggests PRIuS
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=64-bit...)
-- since I have to support Windows as well here, I added a new header to
determine the correct format string.

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:320: return 0;
On 2013/01/14 23:53:41, noelallen1 wrote:
> Isn't this in a response header somewhere?

Done.

https://codereview.chromium.org/11887021/diff/4001/native_client_sdk/src/libr...
native_client_sdk/src/libraries/nacl_mounts/mount_http.cc:330: return
MountNode::Init(mode, uid, gid);
On 2013/01/14 23:53:41, noelallen1 wrote:
> This always returns true...  Which means an open of a non-existent URL will
> appear to be a failed Read.

I've changed this to do an initial stat (which does a HEAD request, should be
quick) to determine if the file exists.

[noelallen1, 2013-01-18 04:13:08]

lgtm

[commit-bot: I haz the power, 2013-01-19 01:20:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/binji@chromium.org/11887021/22001

[commit-bot: I haz the power, 2013-01-19 04:05:24]

Change committed as 177823